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
This avoids accidental dereferencing of an invalid memory address by _mesa_align_free when passed pointer is NULL. Also cleaned up different places where it was used, to avoid double check of passed pointer. Now it is safe to pass NULL pointer to this function and expect same behavior like free(). Best Regards, Siavash Eliasi. Siavash Eliasi (1): 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(-) -- 1.8.4.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[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
