[libvirt] [PATCH] build: silence stupid gcc warning on STREQ_NULLABLE

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120821/a021537f/attachment-0001.sig>