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 {

Reply via email to