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 <[email protected]> > 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.
