[FFmpeg-devel] [PATCH] ffserver: Fix memory leak and uncheked av_strdup return

[wm4]

On Sun, 22 Nov 2015 19:09:58 +0100
Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Sun, Nov 22, 2015 at 5:16 PM, Derek Buitenhuis
> <derek.buitenhuis at gmail.com> wrote:
> > Signed-off-by: Derek Buitenhuis <derek.buitenhuis at gmail.com>
> > ---
> >  ffserver.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/ffserver.c b/ffserver.c
> > index 7e4f620..64a4a7a 100644
> > --- a/ffserver.c
> > +++ b/ffserver.c
> > @@ -3490,9 +3490,13 @@ static int add_av_stream(FFServerStream *feed, AVStream *st)
> >      fst = add_av_stream1(feed, av, 0);
> >      if (!fst)
> >          return -1;
> > -    if (av_stream_get_recommended_encoder_configuration(st))
> > -        av_stream_set_recommended_encoder_configuration(fst,
> > -            av_strdup(av_stream_get_recommended_encoder_configuration(st)));
> > +    if (av_stream_get_recommended_encoder_configuration(st)) {
> > +        char *tmp = av_strdup(av_stream_get_recommended_encoder_configuration(st));
> > +        if (!tmp)
> > +            return -1;
> > +        av_stream_set_recommended_encoder_configuration(fst, tmp);
> > +        av_free(tmp);
> > +    }
> >      return feed->nb_streams - 1;
> >  }
> >  
> 
> I'm slightly confused by this. If the API doesn't take over the value,
> then you don't need to strdup it, and if it does, you don't need to
> free it?

The function that's called here is generated by:

MAKE_ACCESSORS(AVStream, stream, char *, recommended_encoder_configuration)

This just copies the pointer. So I guess freeing it would generate a
dangling pointer? And if the field was already set, you need to get and
free the previous pointer?

(If this is correct, neither code makes sense. I must be missing
something.)