[PATCH 5/5] dix: don't free stranger pointers inside AllocARGBCursor

Ander Conselvan de Oliveira ander.conselvan-de-oliveira at nokia.com
Wed Apr 6 03:20:44 PDT 2011


On 04/04/2011 08:54 PM, Tiago Vignatti wrote:
> This seems a good convention to follow: if pointers are allocate outside a
> given function, then free there as well when a failure occurs.
> 
> AllocARGBCursor and its callers were mixing up the freeing of resources and
> causing a particular double free inside TileScreenSaver (srcbits and mskbits).
> 
> Signed-off-by: Tiago Vignatti<tiago.vignatti at nokia.com>
> ---
>   dix/cursor.c    |    5 +----
>   dix/dispatch.c  |   12 +++++++++---
>   render/render.c |   12 +++++++++---
>   3 files changed, 19 insertions(+), 10 deletions(-)
> 

[...]

> diff --git a/render/render.c b/render/render.c
> index 8ff8ee6..8e58711 100644
> --- a/render/render.c
> +++ b/render/render.c
> @@ -1706,11 +1706,17 @@ ProcRenderCreateCursor (ClientPtr client)
>   			 GetColor(twocolor[1], 0),
>   			&pCursor, client, stuff->cid);
>       if (rc != Success)
> -	return rc;
> -    if (!AddResource(stuff->cid, RT_CURSOR, (pointer)pCursor))
> -	return BadAlloc;
> +	goto bail;
> +    if (!AddResource(stuff->cid, RT_CURSOR, (pointer)pCursor)) {
> +	rc = BadAlloc;
> +	goto bail;
> +    }
> 
>       return Success;
> +bail:
> +    free(srcbits);
> +    free(mskbits);
> +    return rc;
>   }
> 
>   static int


There are a few return points in ProcRenderCreateCursor that could be eliminated in the same fashion. But maybe this is more appropriate as a follow-up patch.

Anyway,

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira at nokia.com>


Cheers,
Ander


More information about the xorg-devel mailing list