Re: [Mesa-dev] [PATCH] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.
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.
The rest looks fine.
With that fixed: Reviewed-by: Brian Paul bri...@vmware.com
Do you need someone to push patches for you?
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);
Re: [Mesa-dev] [PATCH] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.
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
[Mesa-dev] [PATCH] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.
---
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;
+
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);
--
1.8.4.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
