RFR: 8350880: (zipfs) Add support for read-only zip file systems [v3]

David Beaumont duke at openjdk.org
Thu May 15 17:39:56 UTC 2025


On Wed, 14 May 2025 15:40:41 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix comment based on current behaviour.
>
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 95:
> 
>> 93: 
>> 94:     private static final Set<PosixFilePermission> DEFAULT_PERMISSIONS =
>> 95:         PosixFilePermissions.fromString("rwxrwxrwx");
> 
> I am not convinced this needs to be changed as it becomes a  personal style preference

Possibly not, but it is only used in one place, and anyone reading the code now has the comment saying "If not specified in env, it will return 777." three lines away from the code that uses "rwxrwxrwx" instead of nearly 300.

Plus, there isn't now a *mutable* static final constant which could be accidentally leaked and mutated in the future.

Yes, it's personal preference to a degree, but I feel this is a small, but non-zero, improvement in maintainability, and I would ask someone to do this if I were reviewing a change which added this to the code.

> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 113:
> 
>> 111:     final ZipCoder zc;
>> 112:     private final ZipPath rootdir;
>> 113:     // Start readOnly (safe mode) and maybe reset at end of initialization.
> 
> maybe -> may be

Both work. It "may be reset", but also "maybe we will reset it". I reworded slightly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2091678054
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2091679779


More information about the core-libs-dev mailing list