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

[Robert Ottenhag]

Dmitry,

No, that expects str to have length 4 or longer, which might be false. 
The argument 'len' provides the length.

Frederic,

However, I do not get it, why cannot ordinary strcasecmp(str, "true") be 
used here, is it problem with an input string that is not null 
terminated, or is it some other problem?

Reading the bug, the failing input is "-all=true", but how is that a 
problem, assuming the upper layer code has separated "-all=" from "true" 
before parsing the value?

/Robert


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


-- 
Oracle
Robert Ottenhag | Senior Member of Technical Staff
Phone: +46850630961 | Fax: +46850630911 | Mobile: +46707106161
Oracle Java HotSpot Virtual Machine
ORACLE Sweden | Folkungagatan 122 | SE-116 30 Stockholm

Oracle Svenska AB, Kronborgsgränd 17, S-164 28 KISTA, reg.no. 556254-6746

Green Oracle

Oracle is committed to developing practices and products that help protect the environment
--