Diagnostic command fixes

[Nils Loodin]

Ah, I think that the comment was perhaps not clear. The way we specify command-line options today is like this

java -XX:SomeOption=optionOne=valueOne,optionTwo=valuetwo

ie, the diagnostic command parser would see / parse everything after the first equals sign.

However, with options that are boolean, you don't need the boolOption=value syntax, you only need something that would look like

java -XX:SomeOption=boolOption,optionTwo=valueTwo

But then, str would be null, which would be legal, and not something we could/should guard against..
Hope this was a bit clearer.

/Nils

 
On Feb 14, 2012, at 19:31 , Frederic Parain wrote:

>  31   /* NOTE:Some argument types doesn't require a specifier,
>  32    * such as boolean (for instance "Options=setSomething". Notice the
>  33    * difference between this and "Options=setSomething=true".)
>  34    * In these cases, str will be null. This is perfectly valid.
>  35    * But other argument types will have to do null checks.
>  36    */
> 
> Looks like a bug rather than a feature.
> 
> "Options=setSomething=true" is not a syntax originally supported
> by the DCmdParser.
> 
> "Options,setSomething=true" is correct with the comma separator.
> or
> "Options="setSomething=true" is also correct.
> 
> But "Options=setSomething=true" should have been rejected.
> Unfortunately, this error syntax doesn't seem detected.
> 
> Fred
> 
> On 14/02/2012 18:33, Nils Loodin wrote:
>> Forgot an important point - the testing
>> 
>> First I ran all the testing we have for the tracing framework that makes use of these commands.
>> I have also debugged through with frivolous command lines to make sure we don't do anything bad, like crash.
>> Also I've manually tried to use all the combinations of the command line arguments I can think of that should work, and that shouldn't.
>> 
>> Regards
>> Nils Loodin
>> 
>> On Feb 14, 2012, at 18:29 , Nils Loodin wrote:
>> 
>>> Thanks all for suggestions on improvements.
>>> I have an updated webrev here:
>>> http://cr.openjdk.java.net/~nloodin/7145243/webrev.01/
>>> 
>>> Regards
>>> Nils Loodin
>>> 
>>> PS: Note - Frederic Parain has graciously taken upon himself to sponsor this. Many thanks!
>>> 
>>> DS
>>> 
>>> On Feb 13, 2012, at 22:58 , Nils Loodin wrote:
>>> 
>>>> Hey all!
>>>> 
>>>> The new diagnostic command parser needs some additional specializations for time and bytes, here included.
>>>> Also a few fixes for crashes for some combinations of commandlines.
>>>> 
>>>> Tested by throwing a lot of different arguments on the parser, also by running the tests in sun/tools/jcmd.
>>>> 
>>>> http://cr.openjdk.java.net/~nloodin/7145243/webrev.00/
>>>> 
>>>> I would also need a sponsor to get this in..
>>>> 
>>>> Regards
>>>> Nils Loodin
>>> 
>> 
> 
> -- 
> Frederic Parain - Oracle
> Grenoble Engineering Center - France
> Phone: +33 4 76 18 81 17
> Email: Frederic.Parain at oracle.com