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

Johan Sjölen jsjolen at openjdk.org
Mon Oct 30 10:27:36 UTC 2023


On Mon, 30 Oct 2023 09:55:35 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix Windows build
>
> 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;
>     }
>   }

Ouch, I just realized that we can't differentiate between being provided with the literal number 2 and `MemStatAction::print` anymore since you moved the literal number parsing into this function. That means that we can't set `print_final_memstat_report` after returning from this function.

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

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


More information about the serviceability-dev mailing list