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