6516099: InputStream.skipFully(int k) to skip exactly k bytes

[Sergey Bylokhov]

Hi, Brian.

It looks like the new version will not throw EOFException if the "skip(n)" method will skip more bytes than requested, it is possible for example in FileInputStream.skip(long n);

"<p>This method may skip more bytes than what are remaining in the
      * backing file. This produces no exception and the number of bytes skipped
      * may include some number of bytes that were beyond the EOF of the
      * backing file. Attempting to read from the stream after skipping past
      * the end will result in -1 indicating the end of the file."


On 24/10/2018 12:54, Brian Burkhalter wrote:
> Hi Brent,
> 
> Sadly we had a sort of race condition going on here. Here is one more update which gets rid of excess code, namely the discardNBytes() method:
> 
> http://cr.openjdk.java.net/~bpb/6516099/webrev.03/ <http://cr.openjdk.java.net/~bpb/6516099/webrev.03/>
> 
> This implementation assumes that in most cases skip(n) will actually skip n bytes. For “bad” implementations which do not do so, single bytes are read and discarded until either the requested number of bytes has been skipped or EOF is reached. In the latter situation, an EOFException is thrown. This still assumes that skip(n) itself never returns a negative value. The test Skip.java is also updated so that the fallback path in skipNBytes is tested.
> 
> Please also see comments inline below.
> 
> Thanks,
> 
> Brian
> 
>> On Oct 24, 2018, at 12:08 PM, Brent Christian <brent.christian at oracle.com> wrote:
>>
>> I think that's looking pretty good.  One thing:
>>
>> InputStream.java
>>
>> 515      * @param      n   the number of bytes to be skipped.
>>
>> How about an @param for throwOnEOF, too?
> 
> No longer pertinent.
> 
>> Regarding the tests:
>>
>> NullInputStream.java
>>
>> 237     @Test(groups = "closed")
>> 238     public static void testSkipNBytesClosed() {
>>
>> Can this method also use the "expectedExceptions" attribute of @Test ?
> 
> Need to revisit that.
> 
>> --
>> Skip.java
>>
>> Is there a test for skipNBytes() where the skip() call skips < n bytes ?  I'm thinking about code coverage of:
>>
>> 588             // If not enough skipped, read and discard bytes, failing on EOF
>> 589             if (ns != n) {
>> 590                 discardNBytes(n - ns, true);
>> 591             }
>>
>> If not, maybe MyInputStream could override skip(long), with a mode where it would skip < n bytes.
> 
> This is addressed in the patch version .03.
> 


-- 
Best regards, Sergey.