RFR: 8365467: Fix multiple issues in ExplodedImage

David Beaumont duke at openjdk.org
Fri Aug 29 08:37:44 UTC 2025


On Thu, 21 Aug 2025 19:05:44 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> This PR addresses several issues found while adding unit tests for ExplodedImage.
>> I have added review comments for changes (some of which are a little preferential, but bring the code into line with things like ImageReader in terms of the name choices for variables).
>> Later it is likely that this code will be adapted for the up-coming preview mode support in Valhalla, so having it unit-testable is important.
>
> src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java line 193:
> 
>> 191:      *     path references a file which must be hidden in the node hierarchy.
>> 192:      */
>> 193:     Node createModulesNode(String name, Path path) {
> 
> This and other methods could be `private` if not used outside the class.

Done.

There's little consistency in a lot of the access modifiers in the code around here, so I'm defaulting to "don't touch" since I don't know what is, or is not the accepted style. There are lots of other methods here which could/should be private, and it's probably better to raise a PR to go over the entire package and do a sweep for consistency rather than picking these off one-at-a-time.

> src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java line 195:
> 
>> 193:     Node createModulesNode(String name, Path path) {
>> 194:         assert !nodes.containsKey(name) : "Node must not already exist: " + name;
>> 195:         assert name.startsWith(MODULES) && name.length() > MODULES.length() : "Invalid modules name: " + name;
> 
> startsWith(MODUELES) && name.length(...) might be worth a utility method.
> Only 2 uses though.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26757#discussion_r2309546717
PR Review Comment: https://git.openjdk.org/jdk/pull/26757#discussion_r2309547063


More information about the core-libs-dev mailing list