On Mon, 10 Jun 2024 17:08:58 GMT, Chris Plummer <[email protected]> wrote:
>> @dholmes-ora this is one of yours.
>>
>> This was a tad annoying to fix (fix is simple though), since the jcmd
>> framework has no good way to allow for default parameters that are not used
>> literally. E.g. in this case, the real value for the file name will contain
>> the process pid, which of course cannot be hard-coded.
>>
>> New output:
>>
>>
>> Syntax : System.dump_map [options]
>>
>> Options: (options must be specified using the <key> or <key>=<value> syntax)
>> -H : [optional] Human readable format (BOOLEAN, false)
>> -F : [optional] file path (STRING, vm_memory_map_<pid>.txt)
>
> src/hotspot/share/services/diagnosticCommand.cpp line 1211:
>
>> 1209: } else {
>> 1210: name = _filename.value();
>> 1211: }
>
> This code should be considering if a default it specified or not, in case the
> specified value is identical to what is contained in `default_filename`.
> With the current solution, if the user literally specifies
> vm_memory_map_<pid\>.txt, the <pid> part will be expanded to the actual pid.
> See how [JDK-8323546](https://bugs.openjdk.org/browse/JDK-8323546) handled
> this.
Oops yes should definitely be using _filename.is_set() rather than strcmp.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19596#discussion_r1634365367