Hi Brian,
Thanks for taking the suggestions.
I think some of the statements should be normative for all
implementations and
we're trying too hard to work with a possibly broken skip() implementation.
I would recommend that if skip does not do what is expected, then
an IOException is immediately thrown. There's no need for a second attempt.
That will encourage correction of badly written skip() or overrides of
skipNbytes().
Attached is a patch on top of your webrev with my attempt at
separating normative text and reducing the complexity in the @implSpec.
And the resulting javadoc:
/**
* Skips over and discards exactly {@code n} bytes of data from
this input
* stream. If {@code n} is zero, no bytes are skipped.
* If {@code n} is negative, then no bytes are skipped.
* Subclasses may handle the negative value differently.
*
* This method blocks until the requested number of bytes have been
skipped, end
* of file is reached, or an exception is thrown.
*
* If end of stream is reached before the stream is at the desired
position,
* then an {@code EOFException} is thrown.
*
* <p> If an I/O error occurs, then the input stream may be
* in an inconsistent state. It is strongly recommended that the
* stream be promptly closed if an I/O error occurs.
*
* Subclasses are encouraged to provide a more efficient
implementation of this method.
*
* @implSpec
* If {@code n} is zero or negative, then no bytes are skipped.
* If {@code n} is positive, the default implementation of this method
* invokes {@link #skip(long)} with parameter {@code n}. If the return
* value of {@code skip(n)} is less than {@code n}, then {@link
#read()} is
* invoked repeatedly until the stream is {@code n} bytes beyond its
* position when this method was invoked or end of stream is reached.
*
* If the return value of {@code skip(n)} is greater than {@code n},
* then an {@code IOException} is thrown.
*
* @param n the number of bytes to be skipped.
* @throws EOFException if end of stream is encountered before the
* stream can be positioned {@code n} bytes beyond its
position
* when this method was invoked.
* @throws IOException if the stream cannot be positioned
properly or
* if an I/O error occurs.
*/
Regards, Roger
On 11/06/2018 04:30 PM, Brian Burkhalter wrote:
Hello again,
I updated the patch to address the comments made by Roger and Daniel:
http://cr.openjdk.java.net/~bpb/6516099/webrev.05/
Specifically, the method is implemented in terms of skip() and read() and
attempts to account for the possible behaviors of the former, and most of the
specification is moved to an @implSpec block.
The tests are intentionally not updated until such time as agreement on the
method itself might be achieved.
Thanks,
Brian
# HG changeset patch
# Parent 99af5cdf8326a24927c1928477d384f180e9ab1b
diff --git a/src/java.base/share/classes/java/io/InputStream.java b/src/java.base/share/classes/java/io/InputStream.java
--- a/src/java.base/share/classes/java/io/InputStream.java
+++ b/src/java.base/share/classes/java/io/InputStream.java
@@ -548,37 +548,37 @@ public abstract class InputStream implem
/**
* Skips over and discards exactly {@code n} bytes of data from this input
* stream. If {@code n} is zero, no bytes are skipped.
+ * If {@code n} is negative, then no bytes are skipped.
+ * Subclasses may handle the negative value differently.
+ *
+ * This method blocks until the requested number of bytes have been skipped, end
+ * of file is reached, or an exception is thrown.
+ *
+ * If end of stream is reached before the stream is at the desired position,
+ * then an {@code EOFException} is thrown.
+ *
+ * <p> If an I/O error occurs, then the input stream may be
+ * in an inconsistent state. It is strongly recommended that the
+ * stream be promptly closed if an I/O error occurs.
+ *
+ * Subclasses are encouraged to provide a more efficient implementation of this method.
*
* @implSpec
+ * If {@code n} is zero or negative, then no bytes are skipped.
* If {@code n} is positive, the default implementation of this method
* invokes {@link #skip(long)} with parameter {@code n}. If the return
* value of {@code skip(n)} is less than {@code n}, then {@link #read()} is
* invoked repeatedly until the stream is {@code n} bytes beyond its
- * position when this method was invoked or end of stream is reached. If
- * end of stream is reached before the stream is at the desired position,
- * then an {@code EOFException} is thrown. If the return value of
- * {@code skip(n)} is greater than {@code n}, then a backward skip is
- * attempted by invoking {@code skip()} with a negative parameter. If
- * backward skipping is not supported, or the position of the stream after
- * the backward skip is more than {@code n} bytes beyond its position when
- * this method was invoked, then an {@code IOException} is thrown. If the
- * position of the stream after the backward skip is fewer than {@code n}
- * bytes beyond its position when this method was invoked, then
- * {@code read()} is invoked repeatedly until the stream is at the desired
- * position. If end of stream is reached before the stream is at the
- * desired position, then an {@code EOFException} is thrown. Subclasses
- * are encouraged to provide a more efficient implementation of this method.
- * If {@code n} is negative, then no bytes are skipped. Subclasses may
- * handle the negative value differently. For positive {@code n}, this
- * method blocks until the requested number of bytes have been skipped, end
- * of file is reached, or an exception is thrown.
+ * position when this method was invoked or end of stream is reached.
+ *
+ * If the return value of {@code skip(n)} is greater than {@code n},
+ * then an {@code IOException} is thrown.
*
* @param n the number of bytes to be skipped.
* @throws EOFException if end of stream is encountered before the
* stream can be positioned {@code n} bytes beyond its position
* when this method was invoked.
- * @throws IOException if too many bytes were initially skipped and
- * the stream cannot be positioned properly or
+ * @throws IOException if the stream cannot be positioned properly or
* if an I/O error occurs.
*/
public void skipNBytes(long n) throws IOException {