[Mesa-dev] [PATCH 1/3] Remove useless checks for NULL before freeing

Brian Paul brianp at vmware.com
Mon Dec 8 14:20:13 PST 2014


No, we depend on the MALLOC/FREE debug wrappers, etc. for memory 
debugging on Windows.

-Brian

On 12/08/2014 02:40 PM, Ian Romanick wrote:
> Didn't Gallium also remove the FREE business?
>
> Either way, patch 1 and 2 are
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
> I sent a comment on patch 3.  With that comment resolved in patch 3 or
> as a patch 4, patch 3 is also
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
> On 12/08/2014 11:56 AM, Matt Turner wrote:
>> See commits 5067506e and b6109de3 for the Coccinelle script.
>> ---
>>   src/gallium/auxiliary/util/u_debug_flush.c           | 12 ++++--------
>>   src/gallium/drivers/i915/i915_state.c                | 10 ++++------
>>   src/gallium/drivers/ilo/shader/toy_tgsi.c            |  6 ++----
>>   src/gallium/drivers/nouveau/nv50/nv50_context.c      |  3 +--
>>   src/gallium/drivers/nouveau/nv50/nv84_video.c        |  3 +--
>>   src/gallium/drivers/nouveau/nvc0/nvc0_context.c      |  3 +--
>>   src/gallium/drivers/nouveau/nvc0/nvc0_program.c      |  3 +--
>>   src/gallium/drivers/nouveau/nvc0/nvc0_surface.c      |  3 +--
>>   src/gallium/drivers/r600/r600_isa.c                  | 12 ++++--------
>>   src/gallium/drivers/softpipe/sp_tile_cache.c         |  3 +--
>>   src/gallium/state_trackers/hgl/hgl.c                 |  6 ++----
>>   src/gallium/state_trackers/nine/nine_shader.c        |  6 ++----
>>   src/gallium/state_trackers/nine/pixelshader9.c       |  3 +--
>>   src/gallium/state_trackers/nine/stateblock9.c        |  8 ++++----
>>   src/gallium/state_trackers/nine/swapchain9.c         |  2 +-
>>   src/gallium/state_trackers/nine/vertexdeclaration9.c |  9 +++------
>>   src/gallium/state_trackers/nine/vertexshader9.c      |  3 +--
>>   src/gallium/winsys/svga/drm/vmw_screen_ioctl.c       |  6 ++----
>>   src/mesa/drivers/dri/common/xmlconfig.c              |  3 +--
>>   src/mesa/main/objectlabel.c                          |  7 ++-----
>>   20 files changed, 39 insertions(+), 72 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_debug_flush.c b/src/gallium/auxiliary/util/u_debug_flush.c
>> index fdb248c..cdefca2 100644
>> --- a/src/gallium/auxiliary/util/u_debug_flush.c
>> +++ b/src/gallium/auxiliary/util/u_debug_flush.c
>> @@ -132,8 +132,7 @@ debug_flush_buf_reference(struct debug_flush_buf **dst,
>>      struct debug_flush_buf *fbuf = *dst;
>>
>>      if (pipe_reference(&(*dst)->reference, &src->reference)) {
>> -      if (fbuf->map_frame)
>> -         FREE(fbuf->map_frame);
>> +      FREE(fbuf->map_frame);
>>
>>         FREE(fbuf);
>>      }
>> @@ -146,8 +145,7 @@ debug_flush_item_destroy(struct debug_flush_item *item)
>>   {
>>      debug_flush_buf_reference(&item->fbuf, NULL);
>>
>> -   if (item->ref_frame)
>> -      FREE(item->ref_frame);
>> +   FREE(item->ref_frame);
>>
>>      FREE(item);
>>   }
>> @@ -263,10 +261,8 @@ debug_flush_unmap(struct debug_flush_buf *fbuf)
>>
>>      fbuf->mapped_sync = FALSE;
>>      fbuf->mapped = FALSE;
>> -   if (fbuf->map_frame) {
>> -      FREE(fbuf->map_frame);
>> -      fbuf->map_frame = NULL;
>> -   }
>> +   FREE(fbuf->map_frame);
>> +   fbuf->map_frame = NULL;
>>      pipe_mutex_unlock(fbuf->mutex);
>>   }
>>
>> diff --git a/src/gallium/drivers/i915/i915_state.c b/src/gallium/drivers/i915/i915_state.c
>> index c90fcfd..6ba9646 100644
>> --- a/src/gallium/drivers/i915/i915_state.c
>> +++ b/src/gallium/drivers/i915/i915_state.c
>> @@ -628,12 +628,10 @@ void i915_delete_fs_state(struct pipe_context *pipe, void *shader)
>>      FREE(ifs->decl);
>>      ifs->decl = NULL;
>>
>> -   if (ifs->program) {
>> -      FREE(ifs->program);
>> -      ifs->program = NULL;
>> -      FREE((struct tgsi_token *)ifs->state.tokens);
>> -      ifs->state.tokens = NULL;
>> -   }
>> +   FREE(ifs->program);
>> +   ifs->program = NULL;
>> +   FREE((struct tgsi_token *)ifs->state.tokens);
>> +   ifs->state.tokens = NULL;
>>
>>      ifs->program_len = 0;
>>      ifs->decl_len = 0;
>> diff --git a/src/gallium/drivers/ilo/shader/toy_tgsi.c b/src/gallium/drivers/ilo/shader/toy_tgsi.c
>> index 57501ea..65e47bf 100644
>> --- a/src/gallium/drivers/ilo/shader/toy_tgsi.c
>> +++ b/src/gallium/drivers/ilo/shader/toy_tgsi.c
>> @@ -2296,10 +2296,8 @@ add_imm(struct toy_tgsi *tgsi, enum toy_type type, const uint32_t *buf)
>>               cur_size * sizeof(new_types[0]),
>>               new_size * sizeof(new_types[0]));
>>         if (!new_buf || !new_types) {
>> -         if (new_buf)
>> -            FREE(new_buf);
>> -         if (new_types)
>> -            FREE(new_types);
>> +         FREE(new_buf);
>> +         FREE(new_types);
>>            return -1;
>>         }
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_context.c b/src/gallium/drivers/nouveau/nv50/nv50_context.c
>> index 1a53579..2cfd5db 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_context.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_context.c
>> @@ -338,8 +338,7 @@ out_err:
>>         nouveau_bufctx_del(&nv50->bufctx_3d);
>>      if (nv50->bufctx)
>>         nouveau_bufctx_del(&nv50->bufctx);
>> -   if (nv50->blit)
>> -      FREE(nv50->blit);
>> +   FREE(nv50->blit);
>>      FREE(nv50);
>>      return NULL;
>>   }
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv84_video.c b/src/gallium/drivers/nouveau/nv50/nv84_video.c
>> index 395bd7a..7a4670f 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv84_video.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv84_video.c
>> @@ -256,8 +256,7 @@ nv84_decoder_destroy(struct pipe_video_codec *decoder)
>>
>>      nouveau_client_del(&dec->client);
>>
>> -   if (dec->mpeg12_bs)
>> -      FREE(dec->mpeg12_bs);
>> +   FREE(dec->mpeg12_bs);
>>      FREE(dec);
>>   }
>>
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>> index 3992460..7662fb5 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.c
>> @@ -365,8 +365,7 @@ out_err:
>>            nouveau_bufctx_del(&nvc0->bufctx_cp);
>>         if (nvc0->bufctx)
>>            nouveau_bufctx_del(&nvc0->bufctx);
>> -      if (nvc0->blit)
>> -         FREE(nvc0->blit);
>> +      FREE(nvc0->blit);
>>         FREE(nvc0);
>>      }
>>      return NULL;
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> index 21be8b7..c156e91 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> @@ -783,8 +783,7 @@ nvc0_program_destroy(struct nvc0_context *nvc0, struct nvc0_program *prog)
>>
>>      if (prog->mem)
>>         nouveau_heap_free(&prog->mem);
>> -   if (prog->code)
>> -      FREE(prog->code); /* may be 0 for hardcoded shaders */
>> +   FREE(prog->code); /* may be 0 for hardcoded shaders */
>>      FREE(prog->immd_data);
>>      FREE(prog->relocs);
>>      if (prog->type == PIPE_SHADER_COMPUTE && prog->cp.syms)
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
>> index 17fe66d..980fd12 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_surface.c
>> @@ -1501,8 +1501,7 @@ nvc0_blitctx_create(struct nvc0_context *nvc0)
>>   void
>>   nvc0_blitctx_destroy(struct nvc0_context *nvc0)
>>   {
>> -   if (nvc0->blit)
>> -      FREE(nvc0->blit);
>> +   FREE(nvc0->blit);
>>   }
>>
>>   void
>> diff --git a/src/gallium/drivers/r600/r600_isa.c b/src/gallium/drivers/r600/r600_isa.c
>> index 81544ca..4ea7103 100644
>> --- a/src/gallium/drivers/r600/r600_isa.c
>> +++ b/src/gallium/drivers/r600/r600_isa.c
>> @@ -89,14 +89,10 @@ int r600_isa_destroy(struct r600_isa *isa) {
>>   	if (!isa)
>>   		return 0;
>>
>> -	if (isa->alu_op2_map)
>> -		free(isa->alu_op2_map);
>> -	if (isa->alu_op3_map)
>> -		free(isa->alu_op3_map);
>> -	if (isa->fetch_map)
>> -		free(isa->fetch_map);
>> -	if (isa->cf_map)
>> -		free(isa->cf_map);
>> +	free(isa->alu_op2_map);
>> +	free(isa->alu_op3_map);
>> +	free(isa->fetch_map);
>> +	free(isa->cf_map);
>>
>>   	free(isa);
>>   	return 0;
>> diff --git a/src/gallium/drivers/softpipe/sp_tile_cache.c b/src/gallium/drivers/softpipe/sp_tile_cache.c
>> index 78534b5..b763f52 100644
>> --- a/src/gallium/drivers/softpipe/sp_tile_cache.c
>> +++ b/src/gallium/drivers/softpipe/sp_tile_cache.c
>> @@ -186,8 +186,7 @@ sp_tile_cache_set_surface(struct softpipe_tile_cache *tc,
>>         FREE(tc->transfer_map);
>>         tc->num_maps = 0;
>>
>> -      if (tc->clear_flags)
>> -         FREE(tc->clear_flags);
>> +      FREE(tc->clear_flags);
>>         tc->clear_flags_size = 0;
>>      }
>>
>> diff --git a/src/gallium/state_trackers/hgl/hgl.c b/src/gallium/state_trackers/hgl/hgl.c
>> index ce2ffb1..4d7c479 100644
>> --- a/src/gallium/state_trackers/hgl/hgl.c
>> +++ b/src/gallium/state_trackers/hgl/hgl.c
>> @@ -214,8 +214,7 @@ hgl_destroy_st_manager(struct st_manager *manager)
>>   {
>>   	CALLED();
>>
>> -	if (manager)
>> -		FREE(manager);
>> +	FREE(manager);
>>   }
>>
>>
>> @@ -313,6 +312,5 @@ hgl_destroy_st_visual(struct st_visual* visual)
>>   {
>>   	CALLED();
>>
>> -	if (visual)
>> -		FREE(visual);
>> +	FREE(visual);
>>   }
>> diff --git a/src/gallium/state_trackers/nine/nine_shader.c b/src/gallium/state_trackers/nine/nine_shader.c
>> index 3c39b24..c2a0f4d 100644
>> --- a/src/gallium/state_trackers/nine/nine_shader.c
>> +++ b/src/gallium/state_trackers/nine/nine_shader.c
>> @@ -2767,10 +2767,8 @@ tx_dtor(struct shader_translator *tx)
>>   {
>>       if (tx->num_inst_labels)
>>           FREE(tx->inst_labels);
>> -    if (tx->lconstf)
>> -        FREE(tx->lconstf);
>> -    if (tx->regs.r)
>> -        FREE(tx->regs.r);
>> +    FREE(tx->lconstf);
>> +    FREE(tx->regs.r);
>>       FREE(tx);
>>   }
>>
>> diff --git a/src/gallium/state_trackers/nine/pixelshader9.c b/src/gallium/state_trackers/nine/pixelshader9.c
>> index d567bcb..ac204ff 100644
>> --- a/src/gallium/state_trackers/nine/pixelshader9.c
>> +++ b/src/gallium/state_trackers/nine/pixelshader9.c
>> @@ -98,8 +98,7 @@ NinePixelShader9_dtor( struct NinePixelShader9 *This )
>>       }
>>       nine_shader_variants_free(&This->variant);
>>
>> -    if (This->byte_code.tokens)
>> -        FREE((void *)This->byte_code.tokens); /* const_cast */
>> +    FREE((void *)This->byte_code.tokens); /* const_cast */
>>
>>       FREE(This->lconstf.data);
>>       FREE(This->lconstf.ranges);
>> diff --git a/src/gallium/state_trackers/nine/stateblock9.c b/src/gallium/state_trackers/nine/stateblock9.c
>> index cb096c7..36b5e77 100644
>> --- a/src/gallium/state_trackers/nine/stateblock9.c
>> +++ b/src/gallium/state_trackers/nine/stateblock9.c
>> @@ -60,12 +60,12 @@ NineStateBlock9_dtor( struct NineStateBlock9 *This )
>>
>>       nine_state_clear(state, FALSE);
>>
>> -    if (state->vs_const_f) FREE(state->vs_const_f);
>> -    if (state->ps_const_f) FREE(state->ps_const_f);
>> +    FREE(state->vs_const_f);
>> +    FREE(state->ps_const_f);
>>
>> -    if (state->ff.light) FREE(state->ff.light);
>> +    FREE(state->ff.light);
>>
>> -    if (state->ff.transform) FREE(state->ff.transform);
>> +    FREE(state->ff.transform);
>>
>>       if (This->state.changed.ps_const_f) {
>>           for (r = This->state.changed.ps_const_f; r->next; r = r->next);
>> diff --git a/src/gallium/state_trackers/nine/swapchain9.c b/src/gallium/state_trackers/nine/swapchain9.c
>> index a7868ca..bf87aaf 100644
>> --- a/src/gallium/state_trackers/nine/swapchain9.c
>> +++ b/src/gallium/state_trackers/nine/swapchain9.c
>> @@ -177,7 +177,7 @@ NineSwapChain9_Resize( struct NineSwapChain9 *This,
>>       } else if (mode) {
>>           This->mode = malloc(sizeof(D3DDISPLAYMODEEX));
>>           memcpy(This->mode, mode, sizeof(D3DDISPLAYMODEEX));
>> -    } else if (This->mode) {
>> +    } else {
>>           free(This->mode);
>>           This->mode = NULL;
>>       }
>> diff --git a/src/gallium/state_trackers/nine/vertexdeclaration9.c b/src/gallium/state_trackers/nine/vertexdeclaration9.c
>> index 49a60e3..4aea116 100644
>> --- a/src/gallium/state_trackers/nine/vertexdeclaration9.c
>> +++ b/src/gallium/state_trackers/nine/vertexdeclaration9.c
>> @@ -220,12 +220,9 @@ NineVertexDeclaration9_ctor( struct NineVertexDeclaration9 *This,
>>   void
>>   NineVertexDeclaration9_dtor( struct NineVertexDeclaration9 *This )
>>   {
>> -    if (This->decls)
>> -        FREE(This->decls);
>> -    if (This->elems)
>> -        FREE(This->elems);
>> -    if (This->usage_map)
>> -        FREE(This->usage_map);
>> +    FREE(This->decls);
>> +    FREE(This->elems);
>> +    FREE(This->usage_map);
>>
>>       NineUnknown_dtor(&This->base);
>>   }
>> diff --git a/src/gallium/state_trackers/nine/vertexshader9.c b/src/gallium/state_trackers/nine/vertexshader9.c
>> index d8507b5..3d40d60 100644
>> --- a/src/gallium/state_trackers/nine/vertexshader9.c
>> +++ b/src/gallium/state_trackers/nine/vertexshader9.c
>> @@ -105,8 +105,7 @@ NineVertexShader9_dtor( struct NineVertexShader9 *This )
>>       }
>>       nine_shader_variants_free(&This->variant);
>>
>> -    if (This->byte_code.tokens)
>> -        FREE((void *)This->byte_code.tokens); /* const_cast */
>> +    FREE((void *)This->byte_code.tokens); /* const_cast */
>>
>>       FREE(This->lconstf.data);
>>       FREE(This->lconstf.ranges);
>> diff --git a/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c b/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c
>> index c9a3b75..14c3b20 100644
>> --- a/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c
>> +++ b/src/gallium/winsys/svga/drm/vmw_screen_ioctl.c
>> @@ -248,8 +248,7 @@ vmw_ioctl_gb_surface_create(struct vmw_winsys_screen *vws,
>>      return rep->handle;
>>
>>   out_fail_create:
>> -   if (region)
>> -      FREE(region);
>> +   FREE(region);
>>      return SVGA3D_INVALID_ID;
>>   }
>>
>> @@ -378,8 +377,7 @@ out_fail_ref:
>>      if (needs_unref)
>>         vmw_ioctl_surface_destroy(vws, *handle);
>>   out_fail_req:
>> -   if (region)
>> -      FREE(region);
>> +   FREE(region);
>>      return ret;
>>   }
>>
>> diff --git a/src/mesa/drivers/dri/common/xmlconfig.c b/src/mesa/drivers/dri/common/xmlconfig.c
>> index 8e48522..d69eb01 100644
>> --- a/src/mesa/drivers/dri/common/xmlconfig.c
>> +++ b/src/mesa/drivers/dri/common/xmlconfig.c
>> @@ -312,8 +312,7 @@ static unsigned char parseValue (driOptionValue *v, driOptionType type,
>>   	v->_float = strToF (string, &tail);
>>   	break;
>>         case DRI_STRING:
>> -	if (v->_string)
>> -	    free (v->_string);
>> +	free (v->_string);
>>   	v->_string = strndup(string, STRING_CONF_MAXLEN);
>>   	return GL_TRUE;
>>       }
>> diff --git a/src/mesa/main/objectlabel.c b/src/mesa/main/objectlabel.c
>> index 8efc33e..efa7ba8 100644
>> --- a/src/mesa/main/objectlabel.c
>> +++ b/src/mesa/main/objectlabel.c
>> @@ -45,11 +45,8 @@ static void
>>   set_label(struct gl_context *ctx, char **labelPtr, const char *label,
>>             int length, const char *caller)
>>   {
>> -   if (*labelPtr) {
>> -      /* free old label string */
>> -      free(*labelPtr);
>> -      *labelPtr = NULL;
>> -   }
>> +   free(*labelPtr);
>> +   *labelPtr = NULL;
>>
>>      /* set new label string */
>>      if (label) {
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=ID2EeYDEG9Oet8UWCmFok7s-8tUSxOHraaZjS4IOzog&s=uWkaZqaE0U8FeCgI_SM5_LCNXBt-dOsAfRdEWDeFu1E&e=
>



More information about the mesa-dev mailing list