[Mesa-dev] [PATCH] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.
Siavash Eliasi
siavashserver at gmail.com
Tue Dec 3 21:04:59 PST 2013
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 <brianp at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list