On Mon, 13 Nov 2023 13:33:26 GMT, Thomas Stuefe <stu...@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
