Re: [PATCH] Potential memory leak in argz_replace.c

[Corinna Vinschen <vinschen at redhat dot com>]

Hi David,

On May 10 22:01, David Stacey wrote:
> 
>     * libc/argz/argz_replace.c: Fix potential memory leak.
> 
> 

> --- a/newlib/libc/argz/argz_replace.c	2015-03-10 10:40:06.000000000 +0000
> +++ b/newlib/libc/argz/argz_replace.c	2015-05-10 20:19:28.353985800 +0100
> @@ -71,7 +71,10 @@
>  
>        /* reallocate argz, and copy over the new value. */
>        if(!(*argz = (char *)realloc(*argz, new_argz_len)))
> -        return ENOMEM;
> +        {
> +          free(new_argz);
> +          return ENOMEM;
> +        }
>  
>        memcpy(*argz, new_argz, new_argz_len);
>        *argz_len = new_argz_len;

The patch looks ok to me.  Applied.

However, there appear to be more issues with this function.  E.g., when
allocating new_argz, it's never tested if the allocation worked.  Also,
the expression `*argz = (char *)realloc(*argz, new_argz_len)', when
failing, will overwrite *argz with NULL.  When that happens the caller
potentially loses its (un-free'd) argz pointer with no way to recover.

Care to fix those as well?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgpsNuypsHb5B.pgp
Description: PGP signature