RFR: JDK-8318671: Potential uninitialized uintx value after JDK-8317683 [v5]

[Aleksey Shipilev]

On Mon, 13 Nov 2023 13:33:26 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> When using 'MemStat' CompileCommand, we accidentally register the command if an invalid suboption had been specified. Fixed, added regression test (verified).
>
> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
> 
>  - Merge branch 'master' into JDK-8318671-Potential-uninitialized-uintx-value-after-JDK-8317683
>  - Fix Windows build
>  - remove tab
>  - Make patch more palatable
>  - Merge branch 'openjdk:master' into JDK-8318671-Potential-uninitialized-uintx-value-after-JDK-8317683
>  - JDK-8318671-Potential-uninitialized-uintx-value-after-JDK-8317683

src/hotspot/share/compiler/compilerOracle.cpp line 678:

> 676: // Parse an uintx-based option value. Also takes care of parsing enum values for options that are enums.
> 677: // Returns true if ok, false if the value could not be parsed.
> 678: static bool parseUintxValue(enum CompileCommand option, const char* line, uintx& value, int& bytes_read) {

It is honestly weird to see `parse***Uintx***Value` dealing with enums, and be specialized for `MemStat`. Can you reflow this to match how `MemLimit` does it? https://github.com/openjdk/jdk/blob/7bb1999c51cdfeb020047e1094229fda1ec5a99d/src/hotspot/share/compiler/compilerOracle.cpp#L702

src/hotspot/share/compiler/compilerOracle.cpp line 727:

> 725:       line += bytes_read;
> 726:       register_command(matcher, option, value);
> 727:       return;

Why this `return` removed? All other cases in this file have the `return` after `register_command`, which I assume the style here: once any command is properly matched, return.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16335#discussion_r1392741630
PR Review Comment: https://git.openjdk.org/jdk/pull/16335#discussion_r1392718662