[FFmpeg-devel] [PATCH 6/8] avfilter/show_palette: fix memory leak

[Clément Bœsch]

On Mon, Dec 07, 2015 at 08:26:40AM -0500, Ganesh Ajjanagadde wrote:
> On Sun, Dec 6, 2015 at 8:28 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> > On Sat, Dec 5, 2015 at 6:40 AM, Clément Bœsch <u at pkh.me> wrote:
> >> On Fri, Dec 04, 2015 at 05:56:12PM -0500, Ganesh Ajjanagadde wrote:
> >>> On Fri, Dec 4, 2015 at 5:29 PM, Marton Balint <cus at passwd.hu> wrote:
> >>> >>>>      if ((ret = ff_formats_ref(in , &ctx->inputs[0]->out_formats)) < 0
> >>> >>>> ||
> >>> >>>>          (ret = ff_formats_ref(out, &ctx->outputs[0]->in_formats)) < 0)
> >>> >>>> -        return ret;
> >>> >>>> +        goto fail;
> >>> >>>>      return 0;
> >>> >>>> +fail:
> >>> >>>
> >>> >>>
> >>> >>>> +    av_freep(&in->formats);
> >>> >>>
> >>> >>>
> >>> >>> what if in==NULL?
> >>> >>>
> >>> >>>> +    av_freep(&in);
> >>> >>>
> >>> >>>
> >>> >>>> +    av_freep(&out->formats);
> >>> >>>
> >>> >>>
> >>> >>> ditto
> >>> >>>
> >>> >>>> +    av_freep(&out);
> >>> >>>> +    return ret;
> >>> >>>>  }
> >>> >>
> >>> >>
> >>> >> Fixed locally with an if(in) and similar checks. Also applies to other
> >>> >> patches I sent.
> >>> >
> >>> >
> >>> > Maybe it's just me, but don't we usually use two labels for such cases?
> >>> >
> >>> > E.g.
> >>> >
> >>> > fail1:
> >>> >    av_freep(&in->xxx);
> >>> > fail2:
> >>> >    av_freep(&in);
> >>> >    return ret;
> >>>
> >>> I don't really mind, I personally prefer a single goto as I find it
> >>> logically simpler to analyze.
> >>>
> >>
> >> I also prefer a if branching here.
> >
> > Any further comments, or is it ok to push?
> 
> last call for this one; else will go in later today.
> 

You already have my implicit OK, and I wrote that filter, so go ahead.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151207/736f52fe/attachment.sig>