On 08/21/2012 03:25 AM, Daniel P. Berrange wrote: > On Mon, Aug 20, 2012 at 01:51:49PM -0600, Eric Blake wrote: >> Our existing STRNEQ_NULLABLE() triggered a warning in gcc 4.7 when >> used with a literal NULL argument: >> >> qemumonitorjsontest.c: In function 'testQemuMonitorJSONGetMachines': >> qemumonitorjsontest.c:289:5: error: null argument where non-null required (argument 1) [-Werror=nonnull] >> >> * src/internal.h (STREQ_NULLABLE, STRNEQ_NULLABLE): Avoid gcc 4.7 >> stupidity. >> (NULLSTR): Allow passing compile-time constants via macros. >> --- >> src/internal.h | 24 ++++++++++++++++++------ >> 1 file changed, 18 insertions(+), 6 deletions(-) > > Not sure I entirely understand the GCC black magic, but if it works I think the STREQ_NULLABLE changes were pretty easy (convert NULL to "" in the dead code branches so that the unreached strcmp() no longer complains about a NULL arg), but agree that the NULLSTR changes were more black magic. There, the premise is that a literal (such as NULL or "pc") or a variable with [const] char * are accepted, while all other types fall through to the else branch and cause a link error, which makes it so the compiler will complain if you try to do NULLSTR(char) or NULLSTR(char**). Basically, a form of compiler-enforced type checking if your compiler is decent enough. But you made me think about it more - the cases we want to warn about are _already_ warned thanks to other gcc errors - I quickly tested that with F17 gcc: NULLSTR(virBufferPtr) => error: pointer type mismatch in conditional expression [-Werror] NULLSTR(char) => error: pointer/integer type mismatch in conditional expression [-Werror] while NULLSTR(NULL) and NULLSTR("") got through without warnings all without my gyrations. > for you I'll ACK Thanks; I squashed this in before pushing, since it's simpler and still gave the type safety we originally wanted with the verify_true(): diff --git i/src/internal.h w/src/internal.h index c27ffc5..832a931 100644 --- i/src/internal.h +++ w/src/internal.h @@ -205,22 +205,7 @@ /* * Use this when passing possibly-NULL strings to printf-a-likes. */ -# if __GNUC_PREREQ(4, 6) -char *link_error_due_to_bad_NULLSTR_type(void); -# define NULLSTR(s) \ - ({ \ - const char *_tmp; \ - if (__builtin_constant_p(s) || \ - __builtin_types_compatible_p(typeof(s), char *) || \ - __builtin_types_compatible_p(typeof(s), const char *)) \ - _tmp = (s) ? (s) : "(null)"; \ - else \ - _tmp = link_error_due_to_bad_NULLSTR_type(); \ - _tmp; \ - }) -# else -# define NULLSTR(s) ((s) ? (s) : "(null)") -# endif +# define NULLSTR(s) ((s) ? (s) : "(null)") /** * TODO: -- Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list