[Mesa-dev] [PATCH V2] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.

[Brian Paul]

On 12/03/2013 09:50 PM, Siavash Eliasi wrote:
> Revision 2:
> - Avoiding compile error on MSVC and possible warnings on other compilers.
> - Added comment regards passing NULL pointer being safe.
> ---
>   src/mesa/main/imports.c                | 14 +++++++++-----
>   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, 26 insertions(+), 32 deletions(-)
>
> diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
> index 277e947..4afe156 100644
> --- a/src/mesa/main/imports.c
> +++ b/src/mesa/main/imports.c
> @@ -168,6 +168,8 @@ _mesa_align_calloc(size_t bytes, unsigned long alignment)
>    * \param ptr pointer to the memory to be freed.
>    * The actual address to free is stored in the word immediately before the
>    * address the client sees.
> + * Note that it is legal to pass NULL pointer to this function and will be
> + * handled accordingly.
>    */
>   void
>   _mesa_align_free(void *ptr)
> @@ -177,9 +179,11 @@ _mesa_align_free(void *ptr)
>   #elif defined(_WIN32) && defined(_MSC_VER)
>      _aligned_free(ptr);
>   #else
> -   void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
> -   void *realAddr = *cubbyHole;
> -   free(realAddr);
> +   if (ptr) {
> +      void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
> +      void *realAddr = *cubbyHole;
> +      free(realAddr);
> +   }
>   #endif /* defined(HAVE_POSIX_MEMALIGN) */
>   }
>
> @@ -199,8 +203,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);
>

Thanks for making those fixes.  I tweaked the commit message and pushed 
to master.

-Brian