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

[Matthias Bolte]

2011/4/23 Eric Blake <eblake at redhat.com>:
> 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
>

ACK, to your improved version.

Matthias