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.christ...@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.
