2011/4/23 Eric Blake <[email protected]>: > 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 [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
