[flashrom] [PATCH] Fix flashrom memory leaks

Uwe Hermann uwe at hermann-uwe.de
Fri Sep 18 15:33:55 CEST 2009


On Wed, Sep 16, 2009 at 02:48:18PM +0200, Carl-Daniel Hailfinger wrote:
> The pattern of
> foo = realloc(foo, size);
> causes a memory leak if realloc fails because realloc does not free foo
> in case of failure, but foo will be NULL after a failure, so there is no
> way to access or free the original foo.
> 
> Fix unchecked [mc]alloc return values as well.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Looks good and is build-tested by me. But please see below.

Acked-by: Uwe Hermann <uwe at hermann-uwe.de>


> Index: flashrom-memleak/flashrom.c
> ===================================================================
> --- flashrom-memleak/flashrom.c	(Revision 725)
> +++ flashrom-memleak/flashrom.c	(Arbeitskopie)
> @@ -282,9 +282,12 @@
>  
>  char *strcat_realloc(char *dest, const char *src)
>  {
> -	dest = realloc(dest, strlen(dest) + strlen(src) + 1);
> -	if (!dest)
> +	char *newdest = realloc(dest, strlen(dest) + strlen(src) + 1);
> +	if (!newdest) {
> +		free(dest);
> +		fprintf(stderr, "Could not allocate memory!\n");
>  		return NULL;
> +	}
>  	strcat(dest, src);
>  	return dest;

This looks strange, newdest is the new string but we still use dest in
the strcat below? Is that correct?


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org




More information about the flashrom mailing list