On Fri, 20 Feb 2026 12:28:14 GMT, Jaikiran Pai <[email protected]> wrote:
>> Eirik Bjørsnøs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove null check of final field istreams which is never null
>
> src/java.base/share/classes/java/util/zip/ZipFile.java line 736:
>
>> 734: inf.reset();
>> 735: inflaters.addLast(inf);
>> 736: return;
>
> Hello Eirik, nit - my preference here would be to use `add(...)`, which is
> the same as `addLast(...)` but feels a bit more natural when using a `List`.
> When reading the code, the presence of `addLast(...)` makes me stop for a
> while to see if there's anything more going on here other than just adding to
> a `List`.
I probably did this to balance with the `removeLast` in `getInflater`. And to
express the List being used as a FIFO stack.
But I agree this probably raises more questions than answers, so reverted to
add.
> src/java.base/share/classes/java/util/zip/ZipFile.java line 755:
>
>> 753: Inflater inf;
>> 754: while (!inflaters.isEmpty()) {
>> 755: inflaters.removeLast().end();
>
> I think we should replace this with a trivial:
>
>
> for (Inflater inf : inflaters) {
> inf.end();
> }
>
> for the same reasons as in my previous comment - it makes it trivial to
> understand without raising the questions about last vs first.
Good point, yes the iteration order should not matter.
The list will not be empty after this loop, but it doesn't really matter since
it is immediately set to `null` anyhow.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2833008589
PR Review Comment: https://git.openjdk.org/jdk/pull/29430#discussion_r2833023224
