On Mon, 12 May 2025 09:49:12 GMT, David Beaumont <d...@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
