_dbus_string_tolower_ascii() was [Commits to core *must* have reviewers]
ralf.habacker at freenet.de
Wed Feb 24 01:04:37 PST 2010
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.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
More information about the dbus