RFR (XS) : 7131346 Parsing of boolean arguments to diagnostic commands is broken

[Daniel D. Daugherty]

src/share/vm/services/diagnosticArgument.cpp
     Maybe the following comment before line 65:

     // len is the length of the current token starting at str

However, that's just a suggestion. Thumbs up.

Dan


On 1/20/12 7:45 AM, Frederic Parain wrote:
> Thanks David,
>
> I just need a second reviewer and I'll push the fix.
>
> Fred
>
> On 1/20/12 12:18 PM, David Holmes wrote:
>> On 19/01/2012 10:33 PM, Frederic Parain wrote:
>>> On 01/19/12 01:28 PM, David Holmes wrote:
>>>>> However, I'm not sure that trailing characters are properly
>>>>> detected. Do you consider that as a show stopper?
>>>>
>>>> Depends on exactly what you mean by that. If extra junk characters are
>>>> ignored rather than causing an error that's okay to me.
>>>
>>> They will be silently ignored.
>>
>> Not a showstopper in that case.
>>
>> So this looks good to go.
>>
>> David
>> -----
>>
>>> Fred
>>>
>>>>> Fred
>>>>>
>>>>>>> Fred
>>>>>>>
>>>>>>>> On 01/19/2012 11:48 AM, Dmitry Samersoff wrote:
>>>>>>>>> Frederic,
>>>>>>>>>
>>>>>>>>> Sorry, of course
>>>>>>>>>
>>>>>>>>> strncasecmp(str, "true", 4) == 0&& str[4] == 0
>>>>>>>>>
>>>>>>>>> -Dmitry
>>>>>>>>>
>>>>>>>>> On 2012-01-19 14:26, Frederic Parain wrote:
>>>>>>>>>> strncasecmp(str, "true", 4) == 0 would accept
>>>>>>>>>> arguments like this:
>>>>>>>>>>
>>>>>>>>>> -all=truefalse
>>>>>>>>>>
>>>>>>>>>> which are not valid.
>>>>>>>>>>
>>>>>>>>>> Fred
>>>>>>>>>>
>>>>>>>>>> On 01/19/12 11:22 AM, Dmitry Samersoff wrote:
>>>>>>>>>>> Frederic,
>>>>>>>>>>>
>>>>>>>>>>> I think explicit check for len is not necessary,
>>>>>>>>>>>
>>>>>>>>>>> strncasecmp(str, "true", 4) == 0
>>>>>>>>>>>
>>>>>>>>>>> is enough.
>>>>>>>>>>>
>>>>>>>>>>> -Dmitry
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 2012-01-19 13:59, Frederic Parain wrote:
>>>>>>>>>>>> This is a small fix (2 lines) to fix an issue with the
>>>>>>>>>>>> parsing of boolean arguments by diagnostic commands
>>>>>>>>>>>>
>>>>>>>>>>>> CR is not available on bugs.sun.com yet, but the description
>>>>>>>>>>>> says that the string comparisons to identify "true" or "false"
>>>>>>>>>>>> values doesn't take into account the length of the argument
>>>>>>>>>>>> being parse.
>>>>>>>>>>>>
>>>>>>>>>>>> The suggested fix is:
>>>>>>>>>>>>
>>>>>>>>>>>> --- old/src/share/vm/services/diagnosticArgument.cpp Thu 
>>>>>>>>>>>> Jan 19
>>>>>>>>>>>> 10:36:10 2012
>>>>>>>>>>>> +++ new/src/share/vm/services/diagnosticArgument.cpp Thu 
>>>>>>>>>>>> Jan 19
>>>>>>>>>>>> 10:36:10 2012
>>>>>>>>>>>> @@ -62,9 +62,9 @@
>>>>>>>>>>>> if (len == 0) {
>>>>>>>>>>>> set_value(true);
>>>>>>>>>>>> } else {
>>>>>>>>>>>> - if (strcasecmp(str, "true") == 0) {
>>>>>>>>>>>> + if (len == strlen("true")&& strncasecmp(str, "true", len) ==
>>>>>>>>>>>> 0) {
>>>>>>>>>>>> set_value(true);
>>>>>>>>>>>> - } else if (strcasecmp(str, "false") == 0) {
>>>>>>>>>>>> + } else if (len == strlen("false")&& strncasecmp(str, 
>>>>>>>>>>>> "false",
>>>>>>>>>>>> len)
>>>>>>>>>>>> == 0) {
>>>>>>>>>>>> set_value(false);
>>>>>>>>>>>> } else {
>>>>>>>>>>>> THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(),
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~fparain/7131346/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Fred
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>