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

[Johan Sjölen]

On Thu, 26 Oct 2023 16:17:14 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 incrementally with one additional commit since the last revision:
> 
>   Fix Windows build

Hi,

This looks good to me, I've got a couple of comments (one is just a nit). I'll leave it to the compiler folks for approval.

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

> 652:     IF_ENUM_STRING("print", {
> 653:         value = (uintx)MemStatAction::print;
> 654:         print_final_memstat_report = true;

Pre-existing/nit: Setting a member variable is not part of parsing the `uintx` value, should be done by caller. 

Performing this refactoring would also enable us to to have a loop like this (`Pair` is available in `utilities/pair.hpp`):

```c++
  constexpr const int len = 2;
  Pair<const char*, MemStatAction> actions[len] = {
    Pair<const char*, MemStatAction>("collect", MemStatAction::collect),
    Pair<const char*, MemStatAction>("print",MemStatAction::print)
  };
  for (int i = 0; i < len; i++) {
    auto p = actions[i];
    if (strncmp(line, p.first, strlen(p.first)) == 0) {
      value = p.second;
      return true;
    }
  }

test/hotspot/jtreg/compiler/compilercontrol/commands/MemStatTest.java line 3:

> 1: /*
> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> 3:  * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.

Thanks! But I think we're OK with Redhat taking credit for this one :).

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

PR Review: https://git.openjdk.org/jdk/pull/16335#pullrequestreview-1703695585
PR Review Comment: https://git.openjdk.org/jdk/pull/16335#discussion_r1375945596
PR Review Comment: https://git.openjdk.org/jdk/pull/16335#discussion_r1375968556