[PATCH 2/2] Clean up memory better when GetVisualInfo fails in ProcDbeGetVisualInfo

[Jeremy Huddleston]

Reviewed-by: Jeremy Huddleston <jeremyhu at apple.com>

On Apr 19, 2011, at 19:20, Alan Coopersmith wrote:

> Use calloc to initialize pScrVisInfo array so we don't have to check
> which ones were already initialized when freeing them all.
> 
> On failure, set rc if necessary, and jump to code at end that already
> frees all the necessary allocations and return rc.
> 
> Fixes parfait reported error:
> Error: Memory leak (CWE 401)
>   Memory leak of pointer 'pScrVisInfo' allocated with malloc((count * 16))
>        at line 724 of dbe/dbe.c in function 'ProcDbeGetVisualInfo'.
>          'pScrVisInfo' allocated at line 693 with malloc((count * 16)).
>          pScrVisInfo leaks when rc != 0 at line 710
>              and j >= i at line 716.
> 
> [ This bug was found by the Parfait 0.3.7 bug checking tool.
>  For more information see http://labs.oracle.com/projects/parfait/ ]
> 
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
> dbe/dbe.c |   25 +++++++++++--------------
> 1 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/dbe/dbe.c b/dbe/dbe.c
> index 77b616b..51bbdc6 100644
> --- a/dbe/dbe.c
> +++ b/dbe/dbe.c
> @@ -690,8 +690,7 @@ ProcDbeGetVisualInfo(ClientPtr client)
>     }
> 
>     count = (stuff->n == 0) ? screenInfo.numScreens : stuff->n;
> -    if (!(pScrVisInfo = (XdbeScreenVisualInfo *)malloc(count *
> -                        sizeof(XdbeScreenVisualInfo))))
> +    if (!(pScrVisInfo = calloc(count, sizeof(XdbeScreenVisualInfo))))
>     {
>         free(pDrawables);
> 
> @@ -707,21 +706,16 @@ ProcDbeGetVisualInfo(ClientPtr client)
>         pDbeScreenPriv = DBE_SCREEN_PRIV(pScreen);
> 
> 	rc = XaceHook(XACE_SCREEN_ACCESS, client, pScreen, DixGetAttrAccess);
> -	if ((rc != Success) ||
> -	    !(*pDbeScreenPriv->GetVisualInfo)(pScreen, &pScrVisInfo[i]))
> +        if (rc != Success)
> +            goto freeScrVisInfo;
> +
> +        if (!(*pDbeScreenPriv->GetVisualInfo)(pScreen, &pScrVisInfo[i]))
>         {
>             /* We failed to alloc pScrVisInfo[i].visinfo. */
> +            rc = BadAlloc;
> 
>             /* Free visinfos that we allocated for previous screen infos.*/
> -            for (j = 0; j < i; j++)
> -            {
> -                free(pScrVisInfo[j].visinfo);
> -            }
> -
> -            /* Free pDrawables if we needed to allocate it above. */
> -            free(pDrawables);
> -
> -            return (rc == Success) ? BadAlloc : rc;
> +            goto freeScrVisInfo;
>         }
> 
>         /* Account for n, number of xDbeVisInfo items in list. */
> @@ -790,6 +784,9 @@ ProcDbeGetVisualInfo(ClientPtr client)
>         }
>     }
> 
> +    rc = Success;
> +
> +  freeScrVisInfo:
>     /* Clean up memory. */
>     for (i = 0; i < count; i++)
>     {
> @@ -799,7 +796,7 @@ ProcDbeGetVisualInfo(ClientPtr client)
> 
>     free(pDrawables);
> 
> -    return Success;
> +    return rc;
> 
> } /* ProcDbeGetVisualInfo() */
> 
> -- 
> 1.7.3.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
>