[libvirt] [PATCH] util: warn when passing a non-pointer to VIR_FREE

[Eric Blake]

On 04/22/2011 12:21 PM, Christophe Fergeau wrote:
> There were recently some bugs where VIR_FREE was called with an
> int parameter. Try to affect the value passed to VIR_FREE to
> a const void* so that the compiler gets a chance to emit a warning
> if we didn't pass a pointer to VIR_FREE.
> ---
>  src/util/memory.h |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/src/util/memory.h b/src/util/memory.h
> index 66b4c42..fab425d 100644
> --- a/src/util/memory.h
> +++ b/src/util/memory.h
> @@ -197,7 +197,11 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
>   * Free the memory stored in 'ptr' and update to point
>   * to NULL.
>   */
> -# define VIR_FREE(ptr) virFree(&(ptr))
> +# define VIR_FREE(ptr)                                   \
> +    do {                                                 \
> +        ATTRIBUTE_UNUSED const void *check_type = (ptr); \
> +        virFree(&(ptr));                                 \
> +    } while (0)

That evaluates ptr twice.  We can do better, by exploiting that the
ternary operator can be used to determine the type of an expression
without evaluating it.  Gcc allows 1?(void*)expr:pointer (the resulting
type is void*), but hates 1?(void*)expr:int (promoting to int provokes a
warning):

cc1: warnings being treated as errors
remote.c: In function 'remoteDispatchListNetworks':
remote.c:3684:70: error: pointer/integer type mismatch in conditional
expression

So how about:

diff --git i/src/util/memory.h w/src/util/memory.h
index 66b4c42..d77a295 100644
--- i/src/util/memory.h
+++ w/src/util/memory.h
@@ -1,7 +1,7 @@
 /*
  * memory.c: safer memory allocation
  *
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 Red Hat, Inc.
  * Copyright (C) 2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -197,7 +197,11 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
  * Free the memory stored in 'ptr' and update to point
  * to NULL.
  */
-# define VIR_FREE(ptr) virFree(&(ptr))
+/* The ternary ensures that ptr is a pointer and not an integer type,
+ * while evaluating ptr only once.  For now, we intentionally cast
+ * away const, since a number of callers safely pass const char *.
+ */
+# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) &(ptr) :
(ptr)))


 # if TEST_OOM


-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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