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

[Frederic Parain]

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.

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
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at Oracle.com