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.burkhal...@oracle.com> > wrote: > >> On Oct 16, 2018, at 12:53 PM, Roger Riggs <roger.ri...@oracle.com >> <mailto:roger.ri...@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.
