RFR: 8350880: (zipfs) Add support for read-only zip file systems [v4]
Lance Andersen
lancea at openjdk.org
Fri May 16 15:44:55 UTC 2025
On Mon, 12 May 2025 09:49:12 GMT, David Beaumont <duke at openjdk.org> wrote:
>> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Changes based on review feedback.
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 221:
>
>> 219: }
>> 220: // sm and existence check
>> 221: zfpath.getFileSystem().provider().checkAccess(zfpath, java.nio.file.AccessMode.READ);
>
> Type name clash with existing AccessMode class. Since new enum is private, happy to rename.
So perhaps change the name of the newly added enum
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 259:
>
>> 257:
>> 258: // Pass "this" as a parameter after everything else is set up.
>> 259: this.rootdir = new ZipPath(this, new byte[]{'/'});
>
> This could probably be set above release version etc. but it's a choice of either:
> 1. waiting until everything is set up before passing "this"
> 2. letting an incomplete "this" instance get passed to another class (possible escape risk?)
>
> Though in this case the "incompleteness" is limited to it not being set up for multi-jar reading yet, which absolutely shouldn't affect the root path. It feels safer to me to leave it to the end, or perhaps we should dig in to ZipPath, figure out what it really wants here, and just call a different constructor?
I think it was fine where it was originally given how rootdir is used
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2093248468
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2093276490
More information about the core-libs-dev
mailing list