[Mesa-dev] [PATCH v2] st/mesa: Initialize textures array in st_framebuffer_validate

[Thomas Hellstrom]

On 10/17/2017 04:39 PM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> And just reference pipe_resources to it in the validate callbacks.
>
> Avoids pipe_resource leaks when st_framebuffer_validate ends up calling
> the validate callback multiple times, e.g. when a window is resized.
>
> v2:
> * Use generic stable tag instead of Fixes: tag, since the problem could
>    already happen before the commit referenced in v1 (Thomas Hellstrom)
> * Use memset to initialize the array on the stack instead of allocating
>    the array with os_calloc.
>
> Cc: mesa-stable at lists.freedesktop.org
> Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com> # v1
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>   src/gallium/state_trackers/dri/dri_drawable.c | 4 +---
>   src/gallium/state_trackers/glx/xlib/xm_st.c   | 4 +---
>   src/gallium/state_trackers/hgl/hgl.c          | 4 +---
>   src/gallium/state_trackers/osmesa/osmesa.c    | 1 +
>   src/gallium/state_trackers/wgl/stw_st.c       | 4 +---
>   src/mesa/state_tracker/st_manager.c           | 2 ++
>   6 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/src/gallium/state_trackers/dri/dri_drawable.c b/src/gallium/state_trackers/dri/dri_drawable.c
> index 75a8197d330..d586b7564ef 100644
> --- a/src/gallium/state_trackers/dri/dri_drawable.c
> +++ b/src/gallium/state_trackers/dri/dri_drawable.c
> @@ -99,10 +99,8 @@ dri_st_framebuffer_validate(struct st_context_iface *stctx,
>         return TRUE;
>   
>      /* Set the window-system buffers for the state tracker. */
> -   for (i = 0; i < count; i++) {
> -      out[i] = NULL;
> +   for (i = 0; i < count; i++)
>         pipe_resource_reference(&out[i], textures[statts[i]]);
> -   }
>   
>      return TRUE;
>   }
> diff --git a/src/gallium/state_trackers/glx/xlib/xm_st.c b/src/gallium/state_trackers/glx/xlib/xm_st.c
> index 0c42e653c76..946b5dcff29 100644
> --- a/src/gallium/state_trackers/glx/xlib/xm_st.c
> +++ b/src/gallium/state_trackers/glx/xlib/xm_st.c
> @@ -245,10 +245,8 @@ xmesa_st_framebuffer_validate(struct st_context_iface *stctx,
>         }
>      }
>   
> -   for (i = 0; i < count; i++) {
> -      out[i] = NULL;
> +   for (i = 0; i < count; i++)
>         pipe_resource_reference(&out[i], xstfb->textures[statts[i]]);
> -   }
>   
>      return TRUE;
>   }
> diff --git a/src/gallium/state_trackers/hgl/hgl.c b/src/gallium/state_trackers/hgl/hgl.c
> index 1b702815a3a..bbc477a978c 100644
> --- a/src/gallium/state_trackers/hgl/hgl.c
> +++ b/src/gallium/state_trackers/hgl/hgl.c
> @@ -193,10 +193,8 @@ hgl_st_framebuffer_validate(struct st_context_iface *stctxi,
>   		//}
>   	}
>   
> -	for (i = 0; i < count; i++) {
> -		out[i] = NULL;
> +	for (i = 0; i < count; i++)
>   		pipe_resource_reference(&out[i], buffer->textures[statts[i]]);
> -	}
>   
>   	return TRUE;
>   }
> diff --git a/src/gallium/state_trackers/osmesa/osmesa.c b/src/gallium/state_trackers/osmesa/osmesa.c
> index 2f9558db312..44a0cc43812 100644
> --- a/src/gallium/state_trackers/osmesa/osmesa.c
> +++ b/src/gallium/state_trackers/osmesa/osmesa.c
> @@ -432,6 +432,7 @@ osmesa_st_framebuffer_validate(struct st_context_iface *stctx,
>   
>         templat.format = format;
>         templat.bind = bind;
> +      pipe_resource_reference(&out[i], NULL);
>         out[i] = osbuffer->textures[statts[i]] =
>            screen->resource_create(screen, &templat);
>      }
> diff --git a/src/gallium/state_trackers/wgl/stw_st.c b/src/gallium/state_trackers/wgl/stw_st.c
> index 5e165c89f56..7cf18f0a8b0 100644
> --- a/src/gallium/state_trackers/wgl/stw_st.c
> +++ b/src/gallium/state_trackers/wgl/stw_st.c
> @@ -161,10 +161,8 @@ stw_st_framebuffer_validate(struct st_context_iface *stctx,
>         stwfb->fb->must_resize = FALSE;
>      }
>   
> -   for (i = 0; i < count; i++) {
> -      out[i] = NULL;
> +   for (i = 0; i < count; i++)
>         pipe_resource_reference(&out[i], stwfb->textures[statts[i]]);
> -   }
>   
>      stw_framebuffer_unlock(stwfb->fb);
>   
> diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
> index 50bc3c33c62..047337e22db 100644
> --- a/src/mesa/state_tracker/st_manager.c
> +++ b/src/mesa/state_tracker/st_manager.c
> @@ -190,6 +190,8 @@ st_framebuffer_validate(struct st_framebuffer *stfb,
>      if (stfb->iface_stamp == new_stamp)
>         return;
>   
> +   memset(textures, 0, stfb->num_statts * sizeof(textures[0]));
> +
>      /* validate the fb */
>      do {
>         if (!stfb->iface->validate(&st->iface, stfb->iface, stfb->statts,

Also for v2:

Reviewed-by: Thomas Hellstrom <thellstrom at vmware.com>