On Thu, 9 Oct 2025 19:41:20 GMT, Roger Riggs <[email protected]> wrote:

>> C++ changes for supporting preview mode when preview mode resources (with 
>> new location flags) are available.
>> 
>> At the moment, this code will operate on non-preview jimage files (1.0) and 
>> act as if no preview resources are available by virtue of the default value 
>> for missing attributes being zero (which matches location flags for "normal" 
>> entries).
>
> src/java.base/share/native/libjimage/jimage.cpp line 114:
> 
>> 112:     size_t preview_infix_len = strlen(preview_infix);
>> 113: 
>> 114:     // TBD:   assert(module_name_len > 0 && "module name must be 
>> non-empty");
> 
> TBD: is obsolete?

I don't use multiple white-space, so this wasn't me. It's just line 113 in the 
old code moved.
I don't know enough to change it confidently.

> src/java.base/share/native/libjimage/jimage.cpp line 157:
> 
>> 155:     // No preview flags means "a normal resource, without a preview 
>> version".
>> 156:     // This is the overwhelmingly common case, with or without preview 
>> mode.
>> 157:     if (flags == 0) {
> 
> Should test for defined flags, ignoring bits outside of the defined bits.

No, this really needs to be zero in this case, because otherwise it will be 
adding a lot of space to the jimage file for all the attributes that need to be 
encoded.

> src/java.base/share/native/libjimage/jimage.cpp line 164:
> 
>> 162:     if ((flags & ImageLocation::FLAGS_IS_PREVIEW_VERSION) != 0) {
>> 163:         return 0L;
>> 164:     }
> 
> How can this occur?
> classloader is the only client and does not pass arbitrary paths.

Hopefully it can't, but I'm not taking chances.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1618#discussion_r2417833779
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1618#discussion_r2417836907
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1618#discussion_r2417831468

Reply via email to