On 12/03/2013 09:48 PM, Brian Paul wrote:
On 11/26/2013 10:59 PM, Siavash Eliasi wrote:
---
  src/mesa/main/imports.c                |  7 +++++--
  src/mesa/math/m_matrix.c               | 13 +++++--------
  src/mesa/program/prog_parameter.c      |  3 +--
  src/mesa/state_tracker/st_cb_texture.c |  6 ++----
  src/mesa/swrast/s_texture.c            |  7 +++----
  src/mesa/tnl/t_vertex.c                |  6 ++----
  src/mesa/vbo/vbo_exec_api.c            |  9 ++++-----
  7 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 277e947..edfc0d1 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -177,6 +177,9 @@ _mesa_align_free(void *ptr)
  #elif defined(_WIN32) && defined(_MSC_VER)
     _aligned_free(ptr);
  #else
+   if (ptr == NULL)
+      return;
+

We can't have code before declarations like this (MSVC will error and some other compilers warn depending on the C variant being targeted.)

So, how about:

   if (ptr) {
      void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
      void *realAddr = *cubbyHole;
      free(realAddr);
   }

Maybe update the comment on the function to say that passing a NULL pointer is legal.

Thanks for reviewing, all suggested changes are done. Here is the V2 patch containing those changes:

[PATCH V2] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address
http://lists.freedesktop.org/archives/mesa-dev/2013-December/049650.html


The rest looks fine.

With that fixed: Reviewed-by: Brian Paul <bri...@vmware.com>

Do you need someone to push patches for you?

Yes please. I don't have write access.


Best regards,
Siavash Eliasi.



     void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
     void *realAddr = *cubbyHole;
     free(realAddr);
@@ -199,8 +202,8 @@ _mesa_align_realloc(void *oldBuffer, size_t oldSize, size_t newSize,
     if (newBuf && oldBuffer && copySize > 0) {
        memcpy(newBuf, oldBuffer, copySize);
     }
-   if (oldBuffer)
-      _mesa_align_free(oldBuffer);
+
+   _mesa_align_free(oldBuffer);
     return newBuf;
  #endif
  }
diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
index 2902315..274f969 100644
--- a/src/mesa/math/m_matrix.c
+++ b/src/mesa/math/m_matrix.c
@@ -1488,14 +1488,11 @@ _math_matrix_ctr( GLmatrix *m )
  void
  _math_matrix_dtr( GLmatrix *m )
  {
-   if (m->m) {
-      _mesa_align_free( m->m );
-      m->m = NULL;
-   }
-   if (m->inv) {
-      _mesa_align_free( m->inv );
-      m->inv = NULL;
-   }
+   _mesa_align_free( m->m );
+   m->m = NULL;
+
+   _mesa_align_free( m->inv );
+   m->inv = NULL;
  }

  /*@}*/
diff --git a/src/mesa/program/prog_parameter.c b/src/mesa/program/prog_parameter.c
index 4d9cf08..54531d2 100644
--- a/src/mesa/program/prog_parameter.c
+++ b/src/mesa/program/prog_parameter.c
@@ -83,8 +83,7 @@ _mesa_free_parameter_list(struct gl_program_parameter_list *paramList)
        free((void *)paramList->Parameters[i].Name);
     }
     free(paramList->Parameters);
-   if (paramList->ParameterValues)
-      _mesa_align_free(paramList->ParameterValues);
+   _mesa_align_free(paramList->ParameterValues);
     free(paramList);
  }

diff --git a/src/mesa/state_tracker/st_cb_texture.c b/src/mesa/state_tracker/st_cb_texture.c
index faa9ee3..f33d3cf 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -175,10 +175,8 @@ st_FreeTextureImageBuffer(struct gl_context *ctx,
        pipe_resource_reference(&stImage->pt, NULL);
     }

-   if (stImage->TexData) {
-      _mesa_align_free(stImage->TexData);
-      stImage->TexData = NULL;
-   }
+   _mesa_align_free(stImage->TexData);
+   stImage->TexData = NULL;
  }


diff --git a/src/mesa/swrast/s_texture.c b/src/mesa/swrast/s_texture.c
index 27803c5..c08a4e9 100644
--- a/src/mesa/swrast/s_texture.c
+++ b/src/mesa/swrast/s_texture.c
@@ -164,10 +164,9 @@ _swrast_free_texture_image_buffer(struct gl_context *ctx,
                                    struct gl_texture_image *texImage)
  {
struct swrast_texture_image *swImage = swrast_texture_image(texImage);
-   if (swImage->Buffer) {
-      _mesa_align_free(swImage->Buffer);
-      swImage->Buffer = NULL;
-   }
+
+   _mesa_align_free(swImage->Buffer);
+   swImage->Buffer = NULL;

     free(swImage->ImageSlices);
     swImage->ImageSlices = NULL;
diff --git a/src/mesa/tnl/t_vertex.c b/src/mesa/tnl/t_vertex.c
index c7a745e..8c4195e 100644
--- a/src/mesa/tnl/t_vertex.c
+++ b/src/mesa/tnl/t_vertex.c
@@ -546,10 +546,8 @@ void _tnl_free_vertices( struct gl_context *ctx )
        struct tnl_clipspace *vtx = GET_VERTEX_STATE(ctx);
        struct tnl_clipspace_fastpath *fp, *tmp;

-      if (vtx->vertex_buf) {
-         _mesa_align_free(vtx->vertex_buf);
-         vtx->vertex_buf = NULL;
-      }
+      _mesa_align_free(vtx->vertex_buf);
+      vtx->vertex_buf = NULL;

        for (fp = vtx->fastpath ; fp ; fp = tmp) {
           tmp = fp->next;
diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
index 600398c..f3c41e0 100644
--- a/src/mesa/vbo/vbo_exec_api.c
+++ b/src/mesa/vbo/vbo_exec_api.c
@@ -989,11 +989,10 @@ void vbo_use_buffer_objects(struct gl_context *ctx)

     /* Make sure this func is only used once */
     assert(exec->vtx.bufferobj == ctx->Shared->NullBufferObj);
-   if (exec->vtx.buffer_map) {
-      _mesa_align_free(exec->vtx.buffer_map);
-      exec->vtx.buffer_map = NULL;
-      exec->vtx.buffer_ptr = NULL;
-   }
+
+   _mesa_align_free(exec->vtx.buffer_map);
+   exec->vtx.buffer_map = NULL;
+   exec->vtx.buffer_ptr = NULL;

     /* Allocate a real buffer object now */
     _mesa_reference_buffer_object(ctx, &exec->vtx.bufferobj, NULL);


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to