[MPlayer-dev-eng] [PATCH] Useless if NULL before free: libmp{codecs, demux}

[Clément Bœsch]

On Sun, Nov 07, 2010 at 10:00:44PM +0100, Diego Biurrun wrote:
> On Thu, Nov 04, 2010 at 11:19:50PM +0100, Clément Bœsch wrote:
> > 
> > Since the previous patch seems ok, here is the same thing with
> > libmpcodecs/libmpdemux.
> > 
> > I won't send another one anymore since… well you know it's kinda boring.
> > But I think most of them are now dropped for good, so it should improve
> > very slightly the overall quality and avoid a bunch of trivial commits in
> > the future.
> 
> Well, you did send another one anyway, but please commit this one first
> separately so that the size of the changes remains manageable.
> 

The first one does not include all the libmp{codecs,demux}, so I need to
update it. But if I do this split, should I split the full patch for each
subdirectories?

> > --- a/libmpcodecs/vf_hqdn3d.c
> > +++ b/libmpcodecs/vf_hqdn3d.c
> > @@ -45,10 +45,10 @@ struct vf_priv_s {
> >  
> >  static void uninit(struct vf_instance *vf){
> > -	if(vf->priv->Line){free(vf->priv->Line);vf->priv->Line=NULL;}
> > -	if(vf->priv->Frame[0]){free(vf->priv->Frame[0]);vf->priv->Frame[0]=NULL;}
> > -	if(vf->priv->Frame[1]){free(vf->priv->Frame[1]);vf->priv->Frame[1]=NULL;}
> > -	if(vf->priv->Frame[2]){free(vf->priv->Frame[2]);vf->priv->Frame[2]=NULL;}
> > +	free(vf->priv->Line);vf->priv->Line=NULL;
> > +	free(vf->priv->Frame[0]);vf->priv->Frame[0]=NULL;
> > +	free(vf->priv->Frame[1]);vf->priv->Frame[1]=NULL;
> > +	free(vf->priv->Frame[2]);vf->priv->Frame[2]=NULL;
> 
> Please reformat this as
> 
>   free(vf->priv->Line);
>   free(vf->priv->Frame[0]);
>   free(vf->priv->Frame[1]);
>   free(vf->priv->Frame[2]);
> 
>   vf->priv->Line     = NULL;
>   vf->priv->Frame[0] = NULL;
>   vf->priv->Frame[1] = NULL;
>   vf->priv->Frame[2] = NULL;
> 
> while you're at it, the original is quite unreadable IMO.
> 

Clearly, updated :)

> > --- a/libmpdemux/demux_pva.c
> > +++ b/libmpdemux/demux_pva.c
> > @@ -516,11 +516,8 @@ static void demux_seek_pva(demuxer_t * demuxer,float rel_seek_secs,float audio_d
> >  
> >  static void demux_close_pva(demuxer_t * demuxer)
> >  {
> > -	if(demuxer->priv)
> > -	{
> > -		free(demuxer->priv);
> > -		demuxer->priv=NULL;
> > -	}
> > +	free(demuxer->priv);
> > +	demuxer->priv=NULL;
> 
>   demuxer->priv = NULL;
> 
> for extra good karma
> 

Sure.

> > --- a/libmpdemux/demux_realaud.c
> > +++ b/libmpdemux/demux_realaud.c
> > @@ -337,9 +337,8 @@ static void demux_close_ra(demuxer_t *demuxer)
> >  
> >      if (ra_priv) {
> > -	    if (ra_priv->audio_buf)
> > -	        free (ra_priv->audio_buf);
> > -		free(ra_priv);
> > +	free(ra_priv->audio_buf);
> > +	free(ra_priv);
> 
> Indentation looks off.
> 

It is quite logical according to the rest of the file, but reindented
anyway.

-- 
Clément B.
Not sent from a jesusPhone.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Remove-most-of-the-if-p-free-p-all-over-the-code.patch.gz
Type: application/octet-stream
Size: 22978 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20101107/89094bf4/attachment-0001.obj>