RFR: 8332124: Jcmd should recognise options that look like requests for help [v6]

Sonia Zaldana Calles szaldana at openjdk.org
Thu Jul 4 18:15:22 UTC 2024


On Thu, 4 Jul 2024 06:36:41 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Sonia Zaldana Calles has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Updating comments
>>  - Enabling -help to work as well. Updating test cases
>
>> OR, avoid the allocation, we are definitely just reading the characters, so we could use: char _rest = (char_) args.base(); (strtok_r wants a non-const char*)
>> 
>> Then OK great, done.
>> 
> 
> No, please don't do that. It is bad practice - don't get in the habit of casting away constness unless you are sure you know what happens. strtok modifies the string, that is why it needs write access, and it would destroy the string, making it unusable for the case when we have *not* a help command.
> 
> Other than that, it looks okay for me and I will approve it in this form. However, if you have the time, you could rewrite the reorder function to not use strtok but to do the scanning yourself.
> 
> - with a pointer pointing to the start of arguments
> - walk pointer for as long as there are spaces, or until \0
> - then, use strncmp (notice the n) to check for the commands
> - then you are done. No need to copy the string, no need for strtok to filet the string in its whole glory just for most of it to be ignored later. 
> 
> Cheers, Thomas

Thank you both for the reviews! @tstuefe @kevinjwalls

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

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


More information about the serviceability-dev mailing list