[MPlayer-dev-eng] [PATCH] Simplify frees in load_vob_subtitle.

[Clément Bœsch]

On Sat, Feb 05, 2011 at 10:27:52PM +0100, Reimar Döffinger wrote:
> On Sat, Feb 05, 2011 at 10:12:00PM +0100, Clément Bœsch wrote:
> > Hi,
> > 
> > Small patch for the load_vob_subtitle function to cleanup a bit and
> > simplify the ways frees are handled. If you don't mind (maybe you find the
> > code no clearer after that or sth else), I'll commit this in 3 days.
> 
> I am not very convinced yet.
> First, you missed the most obvious one, there is a "free(name); return;" at
> the very beginning that could be replaced by a "goto out".

Indeed, patch updated.

> Next, there is a NULL check missing for the very last mp_path_join.
> 

It does not really matter since the pointer is checked in the
add_subtitles function, but fixed anyway since the add_f function can be
something else.

> Lastly, not sure if it's better or worse, but this
> 
> > @@ -2182,25 +2182,24 @@ void load_vob_subtitle(const char *fname, const char * const ifo, void **spu,
> >              if (!psub)
> >                  goto out;
> >  
> > -            if (add_f(psub, ifo, 0, spu)) {
> > -                free(psub);
> > +            if (add_f(psub, ifo, 0, spu))
> >                  goto out;
> > -            }
> >              free(psub);
> > +            psub = NULL;
> 
> could also be handled like
> res = add_f(psub, ifo, 0, spu);
> free(psub);
> if (res)
>     goto out;

Sure, why not. Patch updated.

Thanks for the review. I'll go for this version if you agree.

-- 
Clément B.