2011/4/23 Eric Blake <eblake@xxxxxxxxxx>:
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list