[FFmpeg-devel] [PATCH] avfilter/formats: fix leak of formats on error

[Michael Niedermayer]

On Tue, Jan 05, 2016 at 01:39:48PM -0800, Ganesh Ajjanagadde wrote:
> On Tue, Jan 5, 2016 at 12:11 PM, Paul B Mahol <onemda at gmail.com> wrote:
> > Signed-off-by: Paul B Mahol <onemda at gmail.com>
> > ---
> >  libavfilter/formats.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> > index a2b19e7..f12dcf4 100644
> > --- a/libavfilter/formats.c
> > +++ b/libavfilter/formats.c
> > @@ -518,6 +518,8 @@ void ff_formats_changeref(AVFilterFormats **oldref, AVFilterFormats **newref)
> >              int ret = ref_fn(fmts, &ctx->inputs[i]->out_fmts);      \
> >              if (ret < 0) {                                          \
> >                  unref_fn(&fmts);                                    \
> > +                av_freep(&fmts->list);                              \
> > +                av_freep(&fmts);                                    \
> >                  return ret;                                         \
> >              }                                                       \
> >              count++;                                                \
> > @@ -528,6 +530,8 @@ void ff_formats_changeref(AVFilterFormats **oldref, AVFilterFormats **newref)
> >              int ret = ref_fn(fmts, &ctx->outputs[i]->in_fmts);      \
> >              if (ret < 0) {                                          \
> >                  unref_fn(&fmts);                                    \
> > +                av_freep(&fmts->list);                              \
> > +                av_freep(&fmts);                                    \
> >                  return ret;                                         \
> >              }                                                       \
> >              count++;                                                \
> 
> This is a good effort, and I favor it. However, note that no matter
> what is done here (ie at the API level in avfilter/formats), it will
> not fix all possible error paths in the filters as far as I can tell.
> The reason roughly boils down to the calls being able to free their
> own stuff on failure, but they are not able to free other things, e.g
> samplerates functions can't free channel layouts.
> 

> Or put in other words, if one wants to be absolutely correct, a ton of
> filters will need updating unfortunately.

i think it would be better to libavfilter cleanup instead of each
filter duplicating cleanup code.
If this needs API extensions somewhere, i dont think thats a problem


> 
> > --
> > 1.9.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160106/5fb94d53/attachment.sig>