From 200ad4d32ef382023c66d139e5e0ad6fb100c844 Mon Sep 17 00:00:00 2001 From: dxu Date: Fri, 17 May 2013 12:04:18 -0700 Subject: [PATCH] 8011136: FileInputStream.available and skip inconsistencies Summary: Correct the behavior of available() and update related java specs for available() and skip() in InputStream and FileInputStream classes. Reviewed-by: alanb --- .../classes/java/io/FileInputStream.java | 21 ++++++++------ src/share/classes/java/io/InputStream.java | 8 ++++-- src/share/native/java/io/FileInputStream.c | 4 ++- .../FileInputStream/LargeFileAvailable.java | 28 ++++++++++--------- .../io/FileInputStream/NegativeAvailable.java | 19 ++++++++----- 5 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/share/classes/java/io/FileInputStream.java b/src/share/classes/java/io/FileInputStream.java index 740b67440..90d1ad5cc 100644 --- a/src/share/classes/java/io/FileInputStream.java +++ b/src/share/classes/java/io/FileInputStream.java @@ -240,13 +240,15 @@ class FileInputStream extends InputStream * *

The skip method may, for a variety of * reasons, end up skipping over some smaller number of bytes, - * possibly 0. If n is negative, an - * IOException is thrown, even though the skip - * method of the {@link InputStream} superclass does nothing in this case. - * The actual number of bytes skipped is returned. + * possibly 0. If n is negative, the method + * will try to skip backwards. In case the backing file does not support + * backward skip at its current position, an IOException is + * thrown. The actual number of bytes skipped is returned. If it skips + * forwards, it returns a positive value. If it skips backwards, it + * returns a negative value. * - *

This method may skip more bytes than are remaining in the backing - * file. This produces no exception and the number of bytes skipped + *

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. @@ -261,9 +263,10 @@ class FileInputStream extends InputStream /** * Returns an estimate of the number of remaining bytes that can be read (or * skipped over) from this input stream without blocking by the next - * invocation of a method for this input stream. The next invocation might be - * the same thread or another thread. A single read or skip of this - * many bytes will not block, but may read or skip fewer bytes. + * invocation of a method for this input stream. Returns 0 when the file + * position is beyond EOF. The next invocation might be the same thread + * or another thread. A single read or skip of this many bytes will not + * block, but may read or skip fewer bytes. * *

In some cases, a non-blocking read (or skip) may appear to be * blocked when it is merely slow, for example when reading large diff --git a/src/share/classes/java/io/InputStream.java b/src/share/classes/java/io/InputStream.java index cb61314b0..3c7ff17f6 100644 --- a/src/share/classes/java/io/InputStream.java +++ b/src/share/classes/java/io/InputStream.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -193,8 +193,10 @@ public abstract class InputStream implements Closeable { * up skipping over some smaller number of bytes, possibly 0. * This may result from any of a number of conditions; reaching end of file * before n bytes have been skipped is only one possibility. - * The actual number of bytes skipped is returned. If n is - * negative, no bytes are skipped. + * The actual number of bytes skipped is returned. If {@code n} is + * negative, the {@code skip} method for class {@code InputStream} always + * returns 0, and no bytes are skipped. Subclasses may handle the negative + * value differently. * *

The skip method of this class creates a * byte array and then repeatedly reads into it until n bytes diff --git a/src/share/native/java/io/FileInputStream.c b/src/share/native/java/io/FileInputStream.c index 37deaf84e..52e2cdd8e 100644 --- a/src/share/native/java/io/FileInputStream.c +++ b/src/share/native/java/io/FileInputStream.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2007, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -100,6 +100,8 @@ Java_java_io_FileInputStream_available(JNIEnv *env, jobject this) { if (IO_Available(fd, &ret)) { if (ret > INT_MAX) { ret = (jlong) INT_MAX; + } else if (ret < 0) { + ret = 0; } return jlong_to_jint(ret); } diff --git a/test/java/io/FileInputStream/LargeFileAvailable.java b/test/java/io/FileInputStream/LargeFileAvailable.java index 90eed30d1..94f6f856c 100644 --- a/test/java/io/FileInputStream/LargeFileAvailable.java +++ b/test/java/io/FileInputStream/LargeFileAvailable.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* * @test - * @bug 6402006 7030573 + * @bug 6402006 7030573 8011136 * @summary Test if available returns correct value when reading * a large file. */ @@ -61,9 +61,12 @@ public class LargeFileAvailable { remaining -= skipBytes(fis, bigSkip, remaining); remaining -= skipBytes(fis, 10L, remaining); remaining -= skipBytes(fis, bigSkip, remaining); - if (fis.available() != (int) remaining) { - throw new RuntimeException("available() returns " - + fis.available() + " but expected " + remaining); + int expected = (remaining >= Integer.MAX_VALUE) + ? Integer.MAX_VALUE + : (remaining > 0 ? (int) remaining : 0); + if (fis.available() != expected) { + throw new RuntimeException("available() returns " + + fis.available() + " but expected " + expected); } } finally { file.delete(); @@ -77,19 +80,18 @@ public class LargeFileAvailable { long skip = is.skip(toSkip); if (skip != toSkip) { throw new RuntimeException("skip() returns " + skip - + " but expected " + toSkip); + + " but expected " + toSkip); } long remaining = avail - skip; - int expected = remaining >= Integer.MAX_VALUE - ? Integer.MAX_VALUE - : (int) remaining; + int expected = (remaining >= Integer.MAX_VALUE) + ? Integer.MAX_VALUE + : (remaining > 0 ? (int) remaining : 0); - System.out.println("Skipped " + skip + " bytes " - + " available() returns " + expected + - " remaining=" + remaining); + System.out.println("Skipped " + skip + " bytes, available() returns " + + expected + ", remaining " + remaining); if (is.available() != expected) { throw new RuntimeException("available() returns " - + is.available() + " but expected " + expected); + + is.available() + " but expected " + expected); } return skip; } diff --git a/test/java/io/FileInputStream/NegativeAvailable.java b/test/java/io/FileInputStream/NegativeAvailable.java index a4dedbcdc..a0e0e9992 100644 --- a/test/java/io/FileInputStream/NegativeAvailable.java +++ b/test/java/io/FileInputStream/NegativeAvailable.java @@ -23,7 +23,7 @@ /* * @test - * @bug 8010837 + * @bug 8010837 8011136 * @summary Test if available returns correct value when skipping beyond * the end of a file. * @author Dan Xu @@ -42,6 +42,7 @@ public class NegativeAvailable { public static void main(String[] args) throws IOException { final int SIZE = 10; final int SKIP = 5; + final int NEGATIVE_SKIP = -5; // Create a temporary file with size of 10 bytes. Path tmp = Files.createTempFile(null, null); @@ -56,12 +57,15 @@ public class NegativeAvailable { try (FileInputStream fis = new FileInputStream(tempFile)) { if (tempFile.length() != SIZE) { throw new RuntimeException("unexpected file size = " - + tempFile.length()); + + tempFile.length()); } long space = skipBytes(fis, SKIP, SIZE); + space = skipBytes(fis, NEGATIVE_SKIP, space); space = skipBytes(fis, SKIP, space); space = skipBytes(fis, SKIP, space); space = skipBytes(fis, SKIP, space); + space = skipBytes(fis, NEGATIVE_SKIP, space); + space = skipBytes(fis, NEGATIVE_SKIP, space); } Files.deleteIfExists(tmp); } @@ -74,17 +78,18 @@ public class NegativeAvailable { long skip = fis.skip(toSkip); if (skip != toSkip) { throw new RuntimeException("skip() returns " + skip - + " but expected " + toSkip); + + " but expected " + toSkip); } - long remaining = space - toSkip; + long newSpace = space - toSkip; + long remaining = newSpace > 0 ? newSpace : 0; int avail = fis.available(); if (avail != remaining) { throw new RuntimeException("available() returns " + avail - + " but expected " + remaining); + + " but expected " + remaining); } System.out.println("Skipped " + skip + " bytes " - + " available() returns " + avail); - return remaining; + + " available() returns " + avail); + return newSpace; } } -- GitLab