Re: [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 

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-sta...@lists.freedesktop.org
Reviewed-by: Thomas Hellstrom  # v1
Signed-off-by: Michel Dänzer 
---
  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([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([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([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([i], NULL);
out[i] = osbuffer->textures[statts[i]] =
   screen->resource_create(screen, );
 }
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([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(>iface, stfb->iface, stfb->statts,


Also for v2:

Reviewed-by: Thomas Hellstrom 


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Michel Dänzer]

From: Michel Dänzer 

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-sta...@lists.freedesktop.org
Reviewed-by: Thomas Hellstrom  # v1
Signed-off-by: Michel Dänzer 
---
 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([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([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([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([i], NULL);
   out[i] = osbuffer->textures[statts[i]] =
  screen->resource_create(screen, );
}
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([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(>iface, stfb->iface, stfb->statts,
-- 
2.15.0.rc0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev