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@redhat.com> --- 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