[libvirt] [PATCH 2/4] viralloc: Honor const correctness in VIR_FREE

[Eric Blake]

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 at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140715/8b9a68f0/attachment-0001.sig>