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

[Ralf Habacker]

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';
> Finally, this patch needs to add some unit tests in
> dbus/dbus-string-util.c (which would have very likely revealed the
> ->constant issue).
>   
Appende is an updated patch.

Regards
 Ralf






-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-_dbus_string_tolower_ascii-new-function.patch
URL: <http://lists.freedesktop.org/archives/dbus/attachments/20100224/66bf13b0/attachment.asc>