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

Radim Vansa rvansa at openjdk.org
Tue Oct 7 12:11:49 UTC 2025


On Fri, 3 Oct 2025 11:23:38 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 146:
> 
>> 144:   while (fgets(line, sizeof(line), f)) {
>> 145:     char *eq = strchr(line, '=');
>> 146:     char *nl = strchr(eq + 1, '\n');
> 
> I believe parsing will be broken if `=` is encountered in the tag name itself. Same for `\n` although that is less likely.

Added extra checks when the tag is added.

> src/java.base/share/native/libcrexec/image_constraints.hpp line 95:
> 
>> 93:   const size_t _max_value_length = 256;
>> 94: 
>> 95:   bool check_tag(const char *name) {
> 
> "check" has too broad meaning IMO
> Suggestion:
> 
>   bool has_tag(const char *name) const {

I kept the `check_tag` name because it's checking more things in the end...

> src/java.base/share/native/libcrexec/linkedlist.hpp line 74:
> 
>> 72:       node = node->_next;
>> 73:     }
>> 74:   }
> 
> I don't insist but implementing an `std::iterator` does not seem too cumbersome and would allow to use the normal for loop

I personally find the lambda more convenient.

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

PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2410406373
PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2410404280
PR Review Comment: https://git.openjdk.org/crac/pull/266#discussion_r2410396963


More information about the crac-dev mailing list