[PATCH:libXfont] Don't leak old allocation if realloc fails to enlarge it

[Peter Hutterer]

On Sun, Dec 08, 2013 at 10:40:15AM -0800, Alan Coopersmith wrote:
> In ftfuncs.c, since the buffer being reallocated is a function local
> buffer, used to accumulate data for a single run of the function and
> then freed at the end of the function, we just free the old buffer if
> realloc fails.
> 
> In atom.c however, the ReverseMap is a static buffer, so we operate in
> temporary variables until we know we're successful, then update the
> static variables.  If we fail, we leave the old static variables in place,
> since they contain data about previous atoms we should maintain, not lose.
> 
> Reported by cppcheck:
> [lib/libXfont/src/FreeType/ftfuncs.c:2122]: (error) Common realloc mistake:
>  'ranges' nulled but not freed upon failure
> [lib/libXfont/src/util/atom.c:126]: (error) Common realloc mistake:
>  'reverseMap' nulled but not freed upon failure
> 
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Cheers,
   Peter

> ---
>  src/FreeType/ftfuncs.c |    9 ++++++---
>  src/util/atom.c        |   20 ++++++++++++--------
>  2 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/src/FreeType/ftfuncs.c b/src/FreeType/ftfuncs.c
> index 2c90cf9..44e5e02 100644
> --- a/src/FreeType/ftfuncs.c
> +++ b/src/FreeType/ftfuncs.c
> @@ -2050,7 +2050,7 @@ restrict_code_range_by_str(int count,unsigned short *refFirstCol,
>  {
>      int nRanges = 0;
>      int result = 0;
> -    fsRange *ranges = NULL;
> +    fsRange *ranges = NULL, *oldRanges;
>      char const *p, *q;
>  
>      p = q = str;
> @@ -2119,10 +2119,13 @@ restrict_code_range_by_str(int count,unsigned short *refFirstCol,
>          fflush(stderr);
>  #endif
>          nRanges++;
> +        oldRanges = ranges;
>          ranges = realloc(ranges, nRanges*sizeof(*ranges));
> -        if (NULL == ranges)
> +        if (NULL == ranges) {
> +            free(oldRanges);
>              break;
> -        {
> +        }
> +        else {
>              fsRange *r = ranges+nRanges-1;
>  
>              r->min_char_low = minpoint & 0xff;
> diff --git a/src/util/atom.c b/src/util/atom.c
> index c47cb5c..37811f9 100644
> --- a/src/util/atom.c
> +++ b/src/util/atom.c
> @@ -118,19 +118,23 @@ ResizeHashTable (void)
>  static int
>  ResizeReverseMap (void)
>  {
> -    int ret = TRUE;
> +    AtomListPtr *newMap;
> +    int newMapSize;
> +
>      if (reverseMapSize == 0)
> -	reverseMapSize = 1000;
> +	newMapSize = 1000;
>      else
> -	reverseMapSize *= 2;
> -    reverseMap = realloc (reverseMap, reverseMapSize * sizeof (AtomListPtr));
> -    if (!reverseMap) {
> +	newMapSize = reverseMapSize * 2;
> +    newMap = realloc (reverseMap, newMapSize * sizeof (AtomListPtr));
> +    if (newMap == NULL) {
>  	fprintf(stderr, "ResizeReverseMap(): Error: Couldn't reallocate"
>  		" reverseMap (%ld)\n",
> -		reverseMapSize * (unsigned long)sizeof(AtomListPtr));
> -	ret = FALSE;
> +		newMapSize * (unsigned long)sizeof(AtomListPtr));
> +	return FALSE;
>      }
> -    return ret;
> +    reverseMap = newMap;
> +    reverseMapSize = newMapSize;
> +    return TRUE;
>  }
>  
>  static int
> -- 
> 1.7.9.2
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>