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

Ganesh Ajjanagadde gajjanag at mit.edu
Mon Dec 7 14:26:40 CET 2015


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.

Remaining CID patches related to avfilter (essentially the same issue
with the same style of fix) will go in 2 days time barring further
comments.

>
>>
>> --
>> 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