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

[David Holmes]

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