[libvirt] [PATCH] Avoid passing NULL to printf %s specifier
# HG changeset patch # User john.levon@sun.com # Date 1231985011 28800 # Node ID 2766ee91dd5ea8e99dac27ce730af0dc46a1d107 # Parent 082e0f7d5de236e69bea177c8d4c4204350144c1 Avoid passing NULL to printf %s specifier This is non-portable. Signed-off-by: John Levon <john.levon@sun.com> 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)") + /** * TODO: * diff --git a/src/libvirt.c b/src/libvirt.c --- a/src/libvirt.c +++ b/src/libvirt.c @@ -874,10 +874,10 @@ do_open (const char *name, " port %d\n" " path %s\n", name, - ret->uri->scheme, ret->uri->opaque, - ret->uri->authority, ret->uri->server, - ret->uri->user, ret->uri->port, - ret->uri->path); + NULLSTR(ret->uri->scheme), NULLSTR(ret->uri->opaque), + NULLSTR(ret->uri->authority), NULLSTR(ret->uri->server), + NULLSTR(ret->uri->user), ret->uri->port, + NULLSTR(ret->uri->path)); } else { DEBUG0("no name, allowing driver auto-select"); }
On Wed, Jan 14, 2009 at 06:03:59PM -0800, john.levon@sun.com wrote:
# HG changeset patch # User john.levon@sun.com # Date 1231985011 28800 # Node ID 2766ee91dd5ea8e99dac27ce730af0dc46a1d107 # Parent 082e0f7d5de236e69bea177c8d4c4204350144c1 Avoid passing NULL to printf %s specifier
This is non-portable.
Signed-off-by: John Levon <john.levon@sun.com>
ACK. Many more of these fixes required throughout libvirt.c and other places for safety sake against callers supplying bogus args. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
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.
participants (4)
-
Daniel P. Berrange -
Jim Meyering -
John Levon -
john.levon＠sun.com