[patch] improved SELinux auditing

Havoc Pennington hp at redhat.com
Mon Nov 8 21:13:14 PST 2004


On Mon, 2004-11-08 at 19:41 -0500, Colin Walters wrote:
> +      const char *dest = dbus_message_get_destination (message);
> +       

Split into "const char *dest;" and "dest = " would make me feel warm and
fuzzy

> +      if (spid)
> +       if (_dbus_string_append (auxdata, " spid="))
> +         _dbus_string_append_uint (auxdata, spid);

Should either do "if (spid && _dbus_string_append)" or add braces for
clarity

>  /**
> + * Copies the contents of a DBusString into a different
> + * buffer. The resulting buffer will be NULL-terminated.

s/NULL/nul/

> +void
> +_dbus_string_copy_to_buffer (const DBusString  *str,
> +                            char              *buffer,
> +                            int                len)
> +{
> +  int buflen;
> +  DBUS_CONST_STRING_PREAMBLE (str);

_dbus_assert (len >= 0) wouldn't hurt. 

> +  buflen = MIN (len, real->len);

real->len is without the nul termination according to comments in dbus-
string-private.h... does that mean the memcpy here does not get a nul if
len > real->len?

The function might be clearer if you name the passed-in len "avail_len"
and buflen something like "copy_len"

> +  memcpy (buffer, real->str, buflen);
> +  if (len > 0 && len == buflen)
> +    buffer[len-1] = '\0';

This won't add the nul if it was skipped in the len > real->len case,
right, since buflen == real->len then.

Havoc




More information about the dbus mailing list