RFR 8194649: Minor cleanup of parameter checking in ByteArrayOutputStream and ObjectInputStream

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR 8194649: Minor cleanup of parameter checking in ByteArrayOutputStream and ObjectInputStream

Brian Burkhalter-2
https://bugs.openjdk.java.net/browse/JDK-8194649

Please see the proposed change included below. In the OIS case the requireNonNull() call is not needed as a NPE would be thrown at the next line anyway if arrayType were null.

Thanks,

Brian

--- a/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
+++ b/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
@@ -27,6 +27,7 @@
 
 import java.nio.charset.Charset;
 import java.util.Arrays;
+import java.util.Objects;
 
 /**
  * This class implements an output stream in which the data is
@@ -147,10 +148,7 @@
      * @param   len   the number of bytes to write.
      */
     public synchronized void write(byte b[], int off, int len) {
-        if ((off < 0) || (off > b.length) || (len < 0) ||
-            ((off + len) - b.length > 0)) {
-            throw new IndexOutOfBoundsException();
-        }
+        Objects.checkFromIndexSize(off, len, b.length);
         ensureCapacity(count + len);
         System.arraycopy(b, off, buf, count, len);
         count += len;

--- a/src/java.base/share/classes/java/io/ObjectInputStream.java
+++ b/src/java.base/share/classes/java/io/ObjectInputStream.java
@@ -1296,7 +1296,6 @@
      * @throws InvalidClassException if the filter rejects creation
      */
     private void checkArray(Class<?> arrayType, int arrayLength) throws InvalidClassException {
-        Objects.requireNonNull(arrayType);
         if (! arrayType.isArray()) {
             throw new IllegalArgumentException("not an array type");
         }
Reply | Threaded
Open this post in threaded view
|

Re: RFR 8194649: Minor cleanup of parameter checking in ByteArrayOutputStream and ObjectInputStream

roger riggs
+1,

(There has been some variation in opinion about use of requireNonNull
even where the VM would check a reference for null; but it does seem
redundant on the face of it, even if hotspot would optimize it)

On 1/4/18 1:14 PM, Brian Burkhalter wrote:

> https://bugs.openjdk.java.net/browse/JDK-8194649
>
> Please see the proposed change included below. In the OIS case the requireNonNull() call is not needed as a NPE would be thrown at the next line anyway if arrayType were null.
>
> Thanks,
>
> Brian
>
> --- a/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
> +++ b/src/java.base/share/classes/java/io/ByteArrayOutputStream.java
> @@ -27,6 +27,7 @@
>  
>   import java.nio.charset.Charset;
>   import java.util.Arrays;
> +import java.util.Objects;
>  
>   /**
>    * This class implements an output stream in which the data is
> @@ -147,10 +148,7 @@
>        * @param   len   the number of bytes to write.
>        */
>       public synchronized void write(byte b[], int off, int len) {
> -        if ((off < 0) || (off > b.length) || (len < 0) ||
> -            ((off + len) - b.length > 0)) {
> -            throw new IndexOutOfBoundsException();
> -        }
> +        Objects.checkFromIndexSize(off, len, b.length);
>           ensureCapacity(count + len);
>           System.arraycopy(b, off, buf, count, len);
>           count += len;
>
> --- a/src/java.base/share/classes/java/io/ObjectInputStream.java
> +++ b/src/java.base/share/classes/java/io/ObjectInputStream.java
> @@ -1296,7 +1296,6 @@
>        * @throws InvalidClassException if the filter rejects creation
>        */
>       private void checkArray(Class<?> arrayType, int arrayLength) throws InvalidClassException {
> -        Objects.requireNonNull(arrayType);
>           if (! arrayType.isArray()) {
>               throw new IllegalArgumentException("not an array type");
>           }