RFR: 8298730: Refactor subsystem_file_line_contents and add docs and tests [v4]
Ioi Lam
iklam at openjdk.org
Tue Jan 3 19:13:04 UTC 2023
On Tue, 27 Dec 2022 12:42:51 GMT, Johan Sjölen <jsjolen 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"`.
>
> Johan Sjölen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 27 additional commits since the last revision:
>
> - Merge remote-tracking branch 'origin/master' into refactor-container
> - Separate success and error cases
> - Add test
> - Inline literal string
> - Move test case to correct spot
> - Add two more test cases
> - Split into single-line and multiple-line tests
> - Rename tests
> - Revert
> - Specify buffer limit in format specifier
> - ... and 17 more: https://git.openjdk.org/jdk/compare/12a98560...5a17527d
I think the current changeset is good. The remaining issues are pre-existing and can be fixed in future RFEs.
-------------
Marked as reviewed by iklam (Reviewer).
PR: https://git.openjdk.org/jdk/pull/11667
More information about the hotspot-runtime-dev
mailing list