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