On 12/14/2011 11:26 PM, Pekka Paalanen wrote:
parcel_out_uniform_storage::visit_field() assigns a strdup()'d string
into gl_uniform_storage::name, but it is never freed.

Free gl_uniform_storage::name, fixes some Valgrind reported memory
leaks.

Signed-off-by: Pekka Paalanen<ppaala...@gmail.com>
---
  src/mesa/main/shaderobj.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 454007f..2275430 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -39,6 +39,7 @@
  #include "program/prog_parameter.h"
  #include "program/hash_table.h"
  #include "ralloc.h"
+#include "../glsl/ir_uniform.h"

  /**********************************************************************/
  /*** Shader object functions                                        ***/
@@ -276,6 +277,9 @@ _mesa_clear_shader_program_data(struct gl_context *ctx,
                                  struct gl_shader_program *shProg)
  {
     if (shProg->UniformStorage) {
+      unsigned i;
+      for (i = 0; i<  shProg->NumUserUniformStorage; ++i)
+         free(shProg->UniformStorage[i].name);
        ralloc_free(shProg->UniformStorage);

This only plugs one of the leaks. If you write a test case that relinks a program, the ralloc_free(UniformStorage) at the top of link_assign_uniform_locations will also leak.

Since the rest of the uniform data is already tracked using ralloc, a better solution is to replace the strdup with ralloc_strdup. Looking in link_uniforms, it seems the names are allocated (via count_uniform_size::process) before the UniformStorage is allocated (via rzalloc_array on line 336). I believe the right answer is:

- Add a 'void *mem_ctx' to the count_uniform_size constructor, and store it in the object.

- Use the mem_ctx in the ralloc_strdup call that will replace the existing strdup call.

 - Pass prog to the count_uniform_size constructor as the mem_ctx.

- At the very end of link_assign_uniform_locations, loop over each entry in UniformStorage and ralloc_reparent the name field to be owned by UniformStorage. This will prevent transient leaks if a program is relinked (due to the ralloc_free at the top of link_assign_uniform_locations).

        shProg->NumUserUniformStorage = 0;
        shProg->UniformStorage = NULL;
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to