_dbus_string_tolower_ascii() was [Commits to core *must* have reviewers]

Ralf Habacker ralf.habacker at freenet.de
Wed Feb 24 12:11:17 PST 2010


Thiago Macieira schrieb:
> Em Quarta-feira 24 Fevereiro 2010, às 10:04:37, Ralf Habacker escreveu:
>   
>> Colin Walters schrieb:
>>     
>>> Hi Ralf,
>>>
>>> On Mon, Feb 22, 2010 at 5:24 AM, Ralf Habacker <ralf.habacker at freenet.de> 
>>>       
> wrote:
>   
>>>> Appended is an updated patch.
>>>>         
>>> Thanks!  There are still some issues.
>>>
>>> First, and this is really important, it's wrong to use
>>> DBUS_CONST_STRING_PREAMBLE here, because we're actually modifying the
>>> string.  DBusString has this useful feature where you can say
>>>
>>> DBusString foo;
>>>
>>> _dbus_string_init_const (&foo, "this string is not copied");
>>>
>>> And not have to handle OOM.  This state is reflected in the ->constant
>>> member.  If we later call
>>>
>>> _dbus_string_tolower_ascii (&foo, 0, 5);
>>>
>>> We'd actually be overwriting static memory, which is not good.  We
>>> either need to use _DBUS_STRING_PREAMBLE which asserts
>>> !string->constant, i.e. make it an error to use with constant data, or
>>> we could check ->constant, and if so duplicate the string (this
>>> implies we need OOM handling, so the function should return a
>>> boolean).  If it's not constant, modify in place.
>>>       
>> thanks for this pointer.
>>
>>     
>>> The second thing is that it's still using islower/toupper, which is
>>> wrong.  It's more correct since we know the string is ASCII to just
>>> say:  Otherwise the function is going to have variable behavior for
>>> say the session bus when someone logs in in a non-ASCII locale.
>>>
>>> if (*s >= 'A' && *s <= 'Z')
>>>
>>>   *s += 32;
>>>       
>> or probably better
>>        *s += 'a' - 'A';
>>
>>     
>
> Or:
> 	*s |= 0x20;
>
>   
yes :-)

which should I take ?

Regards
 Ralf




More information about the dbus mailing list