RFR: 7036144: GZIPInputStream readTrailer uses faulty available() test for end-of-stream [v2]

[Archie Cobbs]

On Thu, 14 Dec 2023 21:14:33 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address review comments.
>
> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 37:
> 
>> 35:     public static void main(String [] args) throws IOException {
>> 36: 
>> 37:         // Create gz data
> 
> Perhaps expand the comment to explain that you are creating a concatenated stream?

Thanks - should be addressed in c7087e55319.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 54:
> 
>> 52:         try (GZIPInputStream in = new GZIPInputStream(new ByteArrayInputStream(gz32))) {
>> 53:             count1 = countBytes(in);
>> 54:         }
> 
> Consider rewriting countBytes to take the ByteArrayInputStream/ZeroAvailableInputStream as a parameter, so you could just do:
> 
>  ```suggestion
>         long count1 = countBytes(new ByteArrayInputStream(gz32));

Thanks - should be addressed in c7087e55319.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 56:
> 
>> 54:         }
>> 55: 
>> 56:         // (a) Read it from a stream where available() always returns zero
> 
> Suggestion:
> 
>         // (b) Read it from a stream where available() always returns zero

Thanks - should be addressed in c7087e55319.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 64:
> 
>> 62:         // They should be the same
>> 63:         if (count2 != count1)
>> 64:             throw new AssertionError(count2 + " != " + count1);
> 
> Consider converting the test to JUnit, this would allow:
> Suggestion:
> 
>         asssertEquals(count1, count2);

Thanks - should be addressed in c7087e55319.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507217
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507107
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507154
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507189