On Thu, 11 Jul 2024 10:39:23 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
>> Jan Kratochvil has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 103 commits: >> >> - Fix the gtest >> - fix compilation warning >> - fix the gtest >> - less refactorizations >> - remove not a real backward compat. >> - whitespace >> - less refactorizations >> - reduce refactorizations >> - Fix caching >> - Merge branch 'master' into master-cgroup >> - ... and 93 more: https://git.openjdk.org/jdk/compare/537d20af...060e7688 > > test/hotspot/gtest/runtime/test_cgroupSubsystem_linux.cpp line 1: > >> 1: /* > > Why are those test changes needed? As before there was for V1: CgroupV1Controller* ctrl = new CgroupV1Controller( (char*)testCases[i]->root_path, (char*)testCases[i]->mount_path, true /* read-only mount */); ctrl->set_subsystem_path((char*)testCases[i]->cgroup_path); vs. for V2: CgroupV2Controller* ctrl = new CgroupV2Controller( (char*)testCases[i]->mount_path, (char*)testCases[i]->cgroup_path, true /* read-only mount */); Which was error prone as both parameters were `char *` with different meaning. Now it is all unified with the same parameters order, for V1: CgroupV1Controller* ctrl = new CgroupV1Controller( (char*)testCases[i]->root_path, (char*)testCases[i]->mount_path, true /* read-only mount */); ctrl->set_subsystem_path((char*)testCases[i]->cgroup_path); and similarly V2: CgroupV2Controller* ctrl = new CgroupV2Controller( (char*)testCases[i]->root_path, (char*)testCases[i]->mount_path, true /* read-only mount */); ctrl->set_subsystem_path((char*)testCases[i]->cgroup_path); That `set_subsystem_path` should be probably a part of the ctor but that is for a later refactorization. Besides that for example `larger_than_max` should be a path but it did not start with a slash ('/') which did not matter as `TestController` did override+skip `subsystem_path()` underneath but then the whole testfile did not test much. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17198#discussion_r1677267509
