Re: 23.0.50; dbus

[Michael Albinus]

Stefan Monnier <address@hidden> writes:

>> Here I'm lost. I use something like
>
>>   if (NILP (uname) || (strlen (SDATA (uname)) > 0))
>
>> uname is a Lisp_Object. SDATA (uname) returns (char *). What kind of
>> signedness do I break in strlen? Does anybody have an idea?
>
> Someone else already answered the question, but I'll just take the
> opportunity to point out that the above code is a common small bug:
> the strlen may crash if uname is neither nil nor a string.
> A better way to write such code is
>
>    if (STRINGP (uname) && (strlen (SDATA (uname)) > 0))

Not really. NILP (uname) is explicitely one condition for the "then"
branch, that's intended. A robust check would be

   if (NILP (uname) || (STRINGP (uname) && (strlen (SDATA (uname)) > 0)))

I'm relatively sure that, if uname is non-nil, it must be a
string. However, an additional check wouldn't hurt.

> Another benefit is that the compiler can do a better job of
> eliminating the (now) redundant STRINGP test that may lurk
> inside SDATA.

Yes. But I still don't know what to do in the MacOS case, where tons of
compiler warnings are raised due to the signedness of the SDATA return
value. Peter has shown it in a recent message. Shall we always cast the
type like "strlen ((char *) SDATA (uname))"? This would affect much more
files but dbusbind.c.

>         Stefan

Best regards, Michael.