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