[Mesa-dev] [PATCH] meta: Plug memory leak in blit shader creation

[Kenneth Graunke]

On Friday, March 06, 2015 06:22:00 PM Ben Widawsky wrote:
> It looks like this has existed since:
> commit f5a477ab76b6e0b268387699cd2253a43db0dfae
> Author: Ian Romanick <ian.d.romanick at intel.com>
> Date:   Mon Dec 16 11:54:08 2013 -0800
> 
>     meta: Refactor shader generation code out of mipmap generation path
> 
> Valgrind was complaining on the piglit test:
> fbo-generatemipmap-formats GL_ARB_texture_float -auto -fbo
> 
> Cc: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Brian Paul <brianp at vmware.com>
> Cc: Eric Anholt <eric at anholt.net>
> Reported-by: Mark Janes <mark.a.janes at intel.com> (Jenkins)
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  src/mesa/drivers/common/meta.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index fdc4cf1..2c1abe3 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -270,6 +270,7 @@ _mesa_meta_setup_blit_shader(struct gl_context *ctx,
>  
>     if (shader->shader_prog != 0) {
>        _mesa_UseProgram(shader->shader_prog);
> +      ralloc_free(mem_ctx);
>        return;
>     }

Hi Ben,

You're right - in the case where the shader already exists (due to us
hitting this path once before), we do create a ralloc context and fail
to free it.  We don't actually need one, since we're not constructing
shader code.

I think it would be better to pull 'mem_ctx = ralloc_context(NULL)' out
of the variable declaration, and put it after this early return.

The early return is the common case, and it'd be nice to avoid
allocating and freeing pointless heap memory for no reason.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150309/7c2dbc45/attachment-0001.sig>