[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