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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org