On 07/15/2014 06:38 AM, Michal Privoznik wrote: > Since we've corrected all the callers that passed a const pointer to > VIR_FREE() we may drop the typecasting horribility and let the macro > be a simple wrapper over the virFree() function. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/util/viralloc.h | 15 +-------------- > 1 file changed, 1 insertion(+), 14 deletions(-) NACK. I agree that we want to fix the macro to not cast away const, but you simplified it too far. > > diff --git a/src/util/viralloc.h b/src/util/viralloc.h > index 7125e67..71b4a45 100644 > --- a/src/util/viralloc.h > +++ b/src/util/viralloc.h > @@ -547,20 +547,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); > * > * This macro is safe to use on arguments with side effects. > */ > -# if !STATIC_ANALYSIS > -/* The ternary ensures that ptr is a pointer and not an integer type, > - * while evaluating ptr only once. This gives us extra compiler > - * safety when compiling under gcc. 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))) This definition was doing two things - casting away const, AND doing type validation. virFree(&foo) will compile even if foo is an int, but we want to ensure that foo is of type char*. I think what you want is: # define VIR_FREE(ptr) virFree(1 ? &(ptr) : (ptr)) > -# else > -/* The Coverity static analyzer considers the else path of the "?:" and > - * flags the VIR_FREE() of the address of the address of memory as a > - * RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(&ptr)) > - */ > -# define VIR_FREE(ptr) virFree((void *) &(ptr)) and keep this alternative to hush up coverity. -- Eric Blake eblake redhat com +1-919-301-3266 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