RFR: 8332124: Jcmd processing should accept the "help" sub option as command argument [v2]

Sonia Zaldana Calles szaldana at openjdk.org
Wed Jun 26 19:53:12 UTC 2024


On Tue, 25 Jun 2024 13:55:32 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

>> Hi all, 
>> 
>> This PR addresses [8332124](https://bugs.openjdk.org/browse/JDK-8332124) enabling jcmd to accept "help" as an argument to subcommands. 
>> 
>> Testing: 
>> - [x] Verified running `jcmd 4711 VM.metaspace help` works along with other subcommands. 
>> - [x] Added test case passes. 
>> 
>> Thanks, 
>> Sonia
>
> Sonia Zaldana Calles has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into JDK-8332124
>  - Adding test case for suboptions with trailing spaces and adding null terminator to reordered command
>  - 8332124: Jcmd processing should accept the "help" sub option as command argument

Hi all, 

Thanks for the comments. Some thoughts: 

>  what about '*jcmd <pid> help <cmd>*'
> not a great deal of additional code for the 'help' jcmd to delegate to
> the target jcmd?

```jcmd <pid> help <cmd>``` is already supported. This patch aims to rearrange the arguments provided to leverage what you suggested  (i.e. ```jcmd <pid> <cmd> help``` -> ```jcmd <pid> help <cmd>```). Not sure if I misunderstood your message though :-) 

Regarding the current state of the patch,
> e.g. "help" can be a filename. We are making the filename "help" unusable. Maybe not a big deal to most people, but we do need to make this clear.

This is a really good point and in my opinion the strongest argument to use `--help` or `-help` instead. My only pushback would be whether it would be a bit inconsistent to make it so that the dashes need to be specified for help in this case ```jcmd <pid> <cmd> -help```, but not in this one ```jcmd <pid> help <cmd>```.

Unless we plan to rework “help” in general and make it so that you always need the dashes regardless of how it gets invoked. 

> 
> jcmd 7537 VM.stringtable help nonsense
> 7537:
> java.lang.IllegalArgumentException: Unknown argument 'help' in diagnostic command.
> ..where I guess it's not the "help" which is unknown. Is the user better served by just giving the help and ignoring the additional "nonsense"?

Sure, that makes sense to me. I can make an update to ignore additional nonsense once we agree on the other points. 

As a final question, if we make it so we need the dashes to enable help, would we need to document this anywhere or do we assume it's self-explanatory as it follows the Posix utility argument syntax that Thomas linked?

Thanks for all the feedback!

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

PR Comment: https://git.openjdk.org/jdk/pull/19776#issuecomment-2192512060


More information about the serviceability-dev mailing list