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