[Mesa-dev] [PATCH 5/9] Remove useless checks for NULL before freeing

[Brian Paul]

On 09/05/2012 08:33 AM, Brian Paul wrote:
> On 09/05/2012 12:42 AM, Matt Turner wrote:
>> This patch has been generated by the following Coccinelle semantic
>> patch:
>>
>> // Remove useless checks for NULL before freeing
>> //
>> // free (NULL) is a no-op, so there is no need to avoid it
>>
>> @@
>> expression E;
>> @@
>> + free (E);
>> + E = NULL;
>> - if (unlikely (E != NULL)) {
>> - free(E);
>> (
>> - E = NULL;
>> |
>> - E = 0;
>> )
>> ...
>> - }
>>
>> @@
>> expression E;
>> type T;
>> @@
>> + free ((T) E);
>> + E = NULL;
>> - if (unlikely (E != NULL)) {
>> - free((T) E);
>> (
>> - E = NULL;
>> |
>> - E = 0;
>> )
>> ...
>> - }
>>
>> @@
>> expression E;
>> @@
>> + free (E);
>> - if (unlikely (E != NULL)) {
>> - free (E);
>> - }
>>
>> @@
>> expression E;
>> type T;
>> @@
>> + free ((T) E);
>> - if (unlikely (E != NULL)) {
>> - free ((T) E);
>> - }
>> ---
>> src/egl/drivers/dri2/egl_dri2.c | 3 +--
>> src/egl/drivers/dri2/platform_x11.c | 3 +--
>> src/egl/drivers/glx/egl_glx.c | 6 ++----
>> src/gallium/drivers/r600/r600_shader.c | 6 ++----
>> src/gallium/state_trackers/egl/x11/glxinit.c | 4 +---
>> src/gallium/state_trackers/egl/x11/x11_screen.c | 9 +++------
>> src/gallium/state_trackers/glx/xlib/xm_api.c | 6 ++----
>> src/gallium/state_trackers/glx/xlib/xm_st.c | 6 ++----
>> src/gallium/state_trackers/vega/shaders_cache.c | 9 +++------
>> src/gallium/state_trackers/xvmc/subpicture.c | 3 +--
>> src/gallium/targets/graw-xlib/graw_xlib.c | 3 +--
>> src/glx/dri2_glx.c | 6 ++----
>> src/glx/dri_glx.c | 6 ++----
>> src/glx/drisw_glx.c | 3 +--
>> src/glx/glxext.c | 9 +++------
>> src/glx/indirect_glx.c | 12 ++++--------
>> src/glx/indirect_vertex_array.c | 12 ++++--------
>> src/mesa/drivers/dri/common/xmlconfig.c | 9 +++------
>> src/mesa/drivers/dri/intel/intel_buffer_objects.c | 6 ++----
>> src/mesa/drivers/dri/intel/intel_tex.c | 6 ++----
>> .../drivers/dri/radeon/radeon_common_context.c | 3 +--
>> src/mesa/drivers/dri/radeon/radeon_texture.c | 6 ++----
>> src/mesa/drivers/x11/fakeglx.c | 4 +---
>> src/mesa/main/atifragshader.c | 12 ++++--------
>> src/mesa/main/bufferobj.c | 3 +--
>> src/mesa/main/context.c | 6 ++----
>> src/mesa/main/imports.c | 3 +--
>> src/mesa/main/shaderapi.c | 4 +---
>> src/mesa/main/shaderobj.c | 9 +++------
>> src/mesa/main/texcompress_cpal.c | 3 +--
>> src/mesa/main/texcompress_fxt1.c | 4 +---
>> src/mesa/program/arbprogparse.c | 12 ++++--------
>> src/mesa/program/nvfragparse.c | 8 ++------
>> src/mesa/program/nvvertparse.c | 8 ++------
>> src/mesa/program/prog_instruction.c | 6 ++----
>> src/mesa/program/prog_parameter.c | 3 +--
>> src/mesa/program/program.c | 3 +--
>> src/mesa/state_tracker/st_cb_texture.c | 3 +--
>> src/mesa/swrast/s_context.c | 3 +--
>> src/mesa/swrast/s_drawpix.c | 4 +---
>> src/mesa/swrast/s_renderbuffer.c | 12 ++++--------
>> src/mesa/swrast/s_texture.c | 6 ++----
>> src/mesa/tnl/t_vb_texgen.c | 4 ++--
>> src/mesa/vbo/vbo_primitive_restart.c | 4 +---
>> src/mesa/vbo/vbo_rebase.c | 6 ++----
>> src/mesa/vbo/vbo_save_api.c | 6 ++----
>> 46 files changed, 88 insertions(+), 184 deletions(-)
>
>
> The patch looks OK (one little note below) but this is pretty huge. In
> the future could you break down big patches into glx, mesa and gallium
> chunks?
>
>
> [...]
>
>> diff --git a/src/gallium/state_trackers/egl/x11/x11_screen.c
>> b/src/gallium/state_trackers/egl/x11/x11_screen.c
>> index 47d9a69..effac0e 100644
>> --- a/src/gallium/state_trackers/egl/x11/x11_screen.c
>> +++ b/src/gallium/state_trackers/egl/x11/x11_screen.c
>> @@ -95,10 +95,8 @@ x11_screen_destroy(struct x11_screen *xscr)
>> {
>> if (xscr->dri_fd>= 0)
>> close(xscr->dri_fd);
>> - if (xscr->dri_driver)
>> - free(xscr->dri_driver);
>> - if (xscr->dri_device)
>> - free(xscr->dri_device);
>> + free(xscr->dri_driver);
>> + free(xscr->dri_device);
>>
>> #ifdef GLX_DIRECT_RENDERING
>> /* xscr->glx_dpy will be destroyed with the X display */
>> @@ -106,8 +104,7 @@ x11_screen_destroy(struct x11_screen *xscr)
>> xscr->glx_dpy->xscr = NULL;
>> #endif
>>
>> - if (xscr->visuals)
>> - free(xscr->visuals);
>> + free(xscr->visuals);
>> FREE(xscr);
>
> Looks like we've still got more MALLOC/CALLOC/FREE clean-up to do.

To elaborate, gallium code still uses MALLOC/FREE but this code is 
using a mix of free/FREE (formerly XFree/FREE).  We should probably 
review that and decide which we really want to use there.

-Brian