[PATCH 6/8] Avoid memory leak on realloc failure in localRegisterFreeBoxCallback

[Peter Hutterer]

On Mon, Jan 28, 2013 at 05:08:40PM -0800, Alan Coopersmith wrote:
> Also avoids leaving invalid pointers in structures if realloc had to
> move them elsewhere to make them larger.
> 
> Found by parfait 1.1 code analyzer:
>    Memory leak of pointer 'newCallbacks' allocated with realloc(((char*)offman->FreeBoxesUpdateCallback), (8 * (offman->NumCallbacks + 1)))
>         at line 328 of hw/xfree86/common/xf86fbman.c in function 'localRegisterFreeBoxCallback'.
>           'newCallbacks' allocated at line 320 with realloc(((char*)offman->FreeBoxesUpdateCallback), (8 * (offman->NumCallbacks + 1))).
>           newCallbacks leaks when newCallbacks != NULL at line 327.
>    Memory leak of pointer 'newPrivates' allocated with realloc(((char*)offman->devPrivates), (8 * (offman->NumCallbacks + 1)))
>         at line 328 of hw/xfree86/common/xf86fbman.c in function 'localRegisterFreeBoxCallback'.
>           'newPrivates' allocated at line 324 with realloc(((char*)offman->devPrivates), (8 * (offman->NumCallbacks + 1))).
>           newPrivates leaks when newCallbacks == NULL at line 327.
> 
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
>  hw/xfree86/common/xf86fbman.c |   12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/xfree86/common/xf86fbman.c b/hw/xfree86/common/xf86fbman.c
> index c2e7bab..4da6af2 100644
> --- a/hw/xfree86/common/xf86fbman.c
> +++ b/hw/xfree86/common/xf86fbman.c
> @@ -320,15 +320,17 @@ localRegisterFreeBoxCallback(ScreenPtr pScreen,
>      newCallbacks = realloc(offman->FreeBoxesUpdateCallback,
>                             sizeof(FreeBoxCallbackProcPtr) *
>                             (offman->NumCallbacks + 1));
> +    if (!newCallbacks)
> +        return FALSE;
> +    else
> +        offman->FreeBoxesUpdateCallback = newCallbacks;
>  
>      newPrivates = realloc(offman->devPrivates,
>                            sizeof(DevUnion) * (offman->NumCallbacks + 1));
> -
> -    if (!newCallbacks || !newPrivates)
> +    if (!newPrivates)
>          return FALSE;

not sure what the callers expect here, but you may be able to free
newCallbacks here too. unless that's cleaned up elsewhere on failure. 

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

Cheers,
   Peter


> -
> -    offman->FreeBoxesUpdateCallback = newCallbacks;
> -    offman->devPrivates = newPrivates;
> +    else
> +        offman->devPrivates = newPrivates;
>  
>      offman->FreeBoxesUpdateCallback[offman->NumCallbacks] = FreeBoxCallback;
>      offman->devPrivates[offman->NumCallbacks].ptr = devPriv;
> -- 
> 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
>