On Fri, 7 Jun 2024 10:40:07 GMT, Thomas Stuefe <stu...@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)

jcmd.l should also have been fixed. It currently reads:


options:

    -H: (Optional) Human readable format (BOOLEAN, false)
    -F: (Optional) File path (STRING, "vm_memory_map_.txt")


Note thet `<pid>` is missing. You can use the changes for 
[JDK-8323546](https://bugs.openjdk.org/browse/JDK-8323546)  as a model on how 
to specify and describe the default parameter.

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.

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

PR Comment: https://git.openjdk.org/jdk/pull/19596#issuecomment-2158908825
PR Review Comment: https://git.openjdk.org/jdk/pull/19596#discussion_r1633575756

Reply via email to