RFR: 8339874: Avoid duplicate checking of trailing slash in ZipFile.getZipEntry [v4]

Eirik Bjørsnøs eirbjo at openjdk.org
Wed Sep 11 07:23:40 UTC 2024


On Tue, 10 Sep 2024 19:34:11 GMT, Claes Redestad <redestad at 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


More information about the core-libs-dev mailing list