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

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

 



john.levon@xxxxxxx 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.

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]