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

Brian Burkhalter brian.burkhalter at oracle.com
Tue Oct 16 20:52:02 UTC 2018


Hi Roger,

Updated patch: http://cr.openjdk.java.net/~bpb/6516099/webrev.01/

Thanks,

Brian

> On Oct 16, 2018, at 1:06 PM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:
> 
>> On Oct 16, 2018, at 12:53 PM, Roger Riggs <Roger.Riggs at oracle.com <mailto:Roger.Riggs at oracle.com>> wrote:
>> 
>> InputStream.java:
>> 
>> 584:  Is there really a case where line 585 would throw the exception?
>> 
>>  skip(n, true) != n
>> 
>> With the 2nd argument = true, it will either skip the bytes or throw.
>> I think you can just call:  "skip(n, true);
> 
> I think you are correct.
> 
>> Given subclasses of InputStream that might override skip(n) with a more efficient mechanism
>> would it make sense to implement skipNBytes in terms of skip(n)?
>> As is, it always uses read() to skip the bytes, which might not be as efficient as possible
>> on some streams (for example a very large file) where a seek could be used.
> 
> That’s a good point. I had some trepidation about the vague wording of skip(n) in terms of how many bytes it actually reads.
> 
>> test/jdk/java/io/InputStream/Skip.java:
>> 
>> There is a lot of whitespace in this test, spaces before "," and extra blank lines.
> 
> Yes it is rather ugly. I chose to leave it as is for initial clarity intending to clean it up later.
> 
>> 97: "possible result" - does that occur in the tests?  The "possible" is a bit ambiguous especially given the very narrow test case. It would be useful to include the message from the exception.
> 
> Copy-paste garbage. I’ll update it to include the message.



More information about the core-libs-dev mailing list