Commits to core *must* have reviewers

[Colin Walters]

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.

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;

Finally, this patch needs to add some unit tests in
dbus/dbus-string-util.c (which would have very likely revealed the
->constant issue).