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

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

 



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@xxxxxxxxxx    +1-801-349-2682
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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]