On Mon, 19 May 2025 12:15:38 GMT, David Beaumont <d...@openjdk.org> wrote:

>> Adding read-only support to ZipFileSystem.
>> 
>> The new `accessMode` environment property allows for readOnly and readWrite 
>> values, and ensures that the requested mode is consistent with what's 
>> returned.
>> 
>> This involved a little refactoring to ensure that "read only" state was set 
>> initially and only unset at the end of initialization if appropriate.
>> 
>> By making 2 methods return values (rather than silently set non-final fields 
>> as a side effect) it's now clear in what order fields are initialized and 
>> which are final (sadly there are still non-final fields, but only a split of 
>> this class into two types can fix that, since determining multi-jar support 
>> requires reading the file system).
>
> David Beaumont has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixed test.

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 210:

> 208:             }
> 209:         }
> 210:         // sm and existence check

Nit - this is pre-existing, but would be good to cleanup. The "sm" here refers 
to SecurityManager. In JDK mainline, the SecurityManager is no longer present. 
So it would be good to change this comment to just "existence check" and remove 
reference to "sm".

src/jdk.zipfs/share/classes/module-info.java line 304:

> 302:  *           </li>
> 303:  *       </ul>
> 304:  *       The access mode has no effect on reported POSIX file 
> permissions (in cases

For consistency, I think this should be:

> The {@code accessMode} has no ....

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095621216
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095616802

Reply via email to