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

Ganesh Ajjanagadde gajjanag at mit.edu
Sun Dec 6 14:28:46 CET 2015


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?

>
> --
> Clément B.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list