[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]