RFR: 8298730: Refactor subsystem_file_line_contents and add docs and tests

[Severin Gehwolf]

On Wed, 14 Dec 2022 11:06:24 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Hi!
>> 
>> Citing the ticket directly:
>> 
>> The function subsystem_file_line_contents does the simple job of parsing lines in a file, but does so in a fairly complex way. This function can be simplified. The function also has a surprising API, as you need to know that in one case the format string needs to take 2 specifiers, instead of 1. 
>> 
>> Some more context:
>> 
>> `subsystem_file_line_contents` either parses files that look like this:
>> 
>> 
>> one_value
>> 
>> 
>> Called as: `subsystem_file_line_contents(ctrl, fname, nullptr, "%s")`
>> 
>> Or like this:
>> 
>> 
>> key1 val1
>> key2 val2
>> 
>> 
>> Called as:  `subsystem_file_line_contents(ctrl, fname, "key1", "%s %s")`
>> 
>> The API for the key/value case is changed to: `subsystem_file_line_contents(ctrl, fname, "key1", "%s")`. Note: `"%s"`, not `"%s %s"`.
>
> test/hotspot/gtest/os/linux/test_cgroupSubsystem_linux.cpp line 137:
> 
>> 135:   err = subsystem_file_line_contents(&my_controller, test_file, "foo", "%s", &s);
>> 136:   EXPECT_EQ(err, 0);
>> 137:   EXPECT_STREQ(s, "1");
> 
> We should have a test where the separator isn't a single space, like `\t` instead. See https://github.com/jerboaa/jdk/commit/8ab1009a18a95510504f70d8a02c822083fdffd1 That's when those tests currently fail. Like I said previously it would also be good to have a test similar to how `CgroupV2Subsystem::cpu_period()` uses it. Like 
> https://github.com/jerboaa/jdk/commit/4d562f08952e17f9137c0da74a3bb587c8539713

I realize you wanted to include this as a separate RFE, but it doesn't make much sense as those additional case are (to me) the bare minimum to have in order to ensure we don't regress.

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

PR: https://git.openjdk.org/jdk/pull/11667