On Tue, 10 Sep 2024 19:34:11 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Eirik Bjørsnøs has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Add whitespace per review feedback
>
> src/java.base/share/classes/java/util/zip/ZipCoder.java line 161:
>
>> 159: }
>> 160:
>> 161: protected boolean hasTrailingSlash(byte[] a, int end) {
>
> Why are you making these `protected`? `ZipCoder` is package-private so any
> inheritors must be in the same package, which means they already have access
> to package-private methods.
Since `hasTrailingSlash` is now only used from UTFCoder and subclasses, I
thought we could stricten the access to protected. But as I learned,
`protected` is still accessible from the same package.
On closer inspection though, `hasTrailingSlash` is now only used from
UTF8ZipEncoder.compare. So we can actually make that implementation `private`
and remove the now-unused implementation from the base class, along with the
`slashBytes` logic. I have pushed these changes.
What do you think?
> src/java.base/share/classes/java/util/zip/ZipFile.java line 1891:
>
>> 1889: // Return the position of "name/" if we found it
>> 1890: if (dirPos != -1) {
>> 1891: return new EntryPos(name +"/", dirPos);
>
> Suggestion:
>
> return new EntryPos(name + "/", dirPos);
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20939#discussion_r1753356858
PR Review Comment: https://git.openjdk.org/jdk/pull/20939#discussion_r1753359793
