On Mon, 10 Jun 2024 17:08:58 GMT, Chris Plummer <cjplum...@openjdk.org> 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

Reply via email to