[lworld] RFR: 8366093: [lworld] Add preview mode to C++ classloader
David Beaumont
duke at openjdk.org
Thu Oct 9 20:08:32 UTC 2025
On Thu, 9 Oct 2025 19:41:20 GMT, Roger Riggs <rriggs at openjdk.org> 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
More information about the valhalla-dev
mailing list