[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Avoid passing NULL to printf %s specifier



john levon sun com wrote:
> Avoid passing NULL to printf %s specifier

This looks fine.
Thanks!

> diff --git a/src/internal.h b/src/internal.h
> --- a/src/internal.h
> +++ b/src/internal.h
> @@ -115,6 +115,8 @@
>  #define ATTRIBUTE_RETURN_CHECK
>  #endif				/* __GNUC__ */
>
> +#define NULLSTR(a) ((a) ? (a) : "(null)")

However, to prevent overzealous developers from accidentally
applying NULLSTR to a non-char*, how about using an inline
function instead?  e.g.,

static inline const char *nullstr(const char *s) { return s ? s : "(null)"; }

Then if I accidentally use "nullstr(some_int_var)", the compiler will
catch it, rather than masking the real error at run time.
Some macro abuses would result in warnings, but at least NULLSTR(0)
would mistakenly hide a real error.  Whoops.  the static inline
function also masks the problem with a literal "0".

A little experimentation shows that using something
like this does most of what I want:

#define NULLSTR(s) \
  ((void)verify_true(sizeof *(s) == sizeof (char)), \
   (s) ? (s) : "(null)")

That verify_true use ensures (at compile time) that "s" is a pointer
to something "char"-sized.  verify_true is defined in gnulib's
#include <verify.h>, which is already used by libvirt.

Then, even NULLSTR(0) is diagnosed at compile time.

For the record, note that this usage does *not* trigger a failure:

    char i[10] = "abcdef";
    printf ("%s", NULLSTR(i));

However, gcc -Wall does print a warning:

    warning: the address of 'i' will always evaluate as 'true'

but that's moot, because there's no reason ever to use NULLSTR
on such a variable.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]