[crac] RFR: 8368929: [CRaC] Move CPUFeatures check to C/R engine [v4]

Timofei Pushkin tpushkin at openjdk.org
Fri Oct 3 12:02:30 UTC 2025


On Fri, 3 Oct 2025 11:05:34 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix NoCPUFeaturesTest
>
> src/java.base/share/native/libcrexec/image_constraints.cpp line 39:
> 
>> 37: 
>> 38: static FILE *open_tags(const char *image_location, const char *mode) {
>> 39:   char fname[PATH_MAX];
> 
> In `crexec.cpp` we for some reason have our own conditional definition of `PATH_MAX`, I guess it caused problems at some point... Maybe create some `common.hpp` or `crexec.hpp` where `PATH_MAX` and `CREXEC` would be defined?

Yep, probably this is the reason: https://github.com/rvansa/crac/actions/runs/18203262060/job/51827234536#step:10:8170

> src/java.base/share/native/libcrexec/image_constraints.hpp line 93:
> 
>> 91: 
>> 92:   const size_t _max_name_length = 256;
>> 93:   const size_t _max_value_length = 256;
> 
> Suggestion:
> 
>   static constexpr size_t _MAX_NAME_LENGTH = 256;
>   static constexpr size_t _MAX_VALUE_LENGTH = 256;

A nitpick, but in the case of labels, where both names and values are strings it may be error-prone that the first refers to a length excluding the nul-char and the second refers to the full size including the nul-char. I usually use "length" for the first and "size" for the second.

> src/java.base/share/native/libcrexec/image_constraints.hpp line 119:
> 
>> 117:       .data = strdup(value),
>> 118:       .data_length = value_length,
>> 119:     });
> 
> `add` and `strdup`s can fail — this should be checked

Error-checking in the functions below is also lacking

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

PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2401591494
PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2401517189
PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2401525482


More information about the crac-dev mailing list