RFR: 8268435: (ch) ChannelInputStream could override readAllBytes [v2]

[Brian Burkhalter]

On Thu, 23 Sep 2021 22:12:52 GMT, delvh <github.com+51889757+delvh at openjdk.org> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8268435: Use instanceof pattern matching
>
> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 115:
> 
>> 113:     @Override
>> 114:     public byte[] readAllBytes() throws IOException {
>> 115:         if (!(ch instanceof SeekableByteChannel))
> 
> I think Pattern matching can be used here to make `117` obsolete. So:
> Suggestion:
> 
>         if (!(ch instanceof SeekableByteChannel sbc))

Fixed.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 117:
> 
>> 115:         if (!(ch instanceof SeekableByteChannel))
>> 116:             return super.readAllBytes();
>> 117:         SeekableByteChannel sbc = (SeekableByteChannel)ch;
> 
> Suggestion:

Fixed.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 126:
> 
>> 124:             return super.readAllBytes();
>> 125: 
>> 126:         if (size > (long) Integer.MAX_VALUE) {
> 
> Suggestion:
> 
>         else if (size > (long) Integer.MAX_VALUE) {

It is common practice in the libraries to use multiple `if` statements to return as soon as feasible.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 138:
> 
>> 136:         int nread = 0;
>> 137:         int n;
>> 138:         for (;;) {
> 
> Suggestion:
> 
>         while (true) {

The use of `for(;;)` is common; not changed.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 164:
> 
>> 162:         if (len < 0)
>> 163:             throw new IllegalArgumentException("len < 0");
>> 164:         if (len == 0)
> 
> Suggestion:
> 
>         else if (len == 0)

Same comment as above.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 167:
> 
>> 165:             return new byte[0];
>> 166: 
>> 167:         if (!(ch instanceof SeekableByteChannel))
> 
> Suggestion:
> 
>         else if (!(ch instanceof SeekableByteChannel sbc))

Fixed.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 169:
> 
>> 167:         if (!(ch instanceof SeekableByteChannel))
>> 168:             return super.readAllBytes();
>> 169:         SeekableByteChannel sbc = (SeekableByteChannel)ch;
> 
> Suggestion:

Fixed.

> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 186:
> 
>> 184:         do {
>> 185:             n = read(buf, nread, remaining);
>> 186:             if (n > 0 ) {
> 
> Formatting?
> Suggestion:
> 
>             if (n > 0) {

Fixed here and in `FileInputStream`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5645