[FFmpeg-devel] x11grab.c: minor clean up, added more documentation

[Reimar Döffinger]

On Fri, Apr 08, 2011 at 06:16:48PM +0200, Michael Niedermayer wrote:
> On Fri, Apr 01, 2011 at 03:56:45PM +0000, Carl Eugen Hoyos wrote:
> > Sven C. Dack <sven.c.dack <at> virginmedia.com> writes:
> > 
> > > >> +    av_free(x11grab->dpyname);
> > > >>      
> > > > So I guess my solution, to free param immediately after the call to
> > > > XOpenDisplay(), was wrong
> > > >    
> > > What are you talking about?
> > 
> > I had the following (inlined) patch in my local tree since I applied "Remove a
> > memory allocation and the associated memcpy.".
> > 
> > diff --git a/libavdevice/x11grab.c b/libavdevice/x11grab.c
> > index 6a674ee..b8fcd1a 100644
> > --- a/libavdevice/x11grab.c
> > +++ b/libavdevice/x11grab.c
> > @@ -106,5 +106,6 @@
> > 
> >      dpy = XOpenDisplay(param);
> > +    av_free(param);
> >      if(!dpy) {
> >          av_log(s1, AV_LOG_ERROR, "Could not open X display.\n");
> >          return AVERROR(EIO);
> > 
> > I have no idea if it is correct, but it fixed the memleak for me (and does not
> > crash here).
> > Is it wrong?
> 
> i applied it, if its wrong then so are other projects also ive not
> found a document that said the string has to be preserved until point
> X.
> Maybe someone who knows x11 api could clarify this

Since it's not documented it's not required by the API (and it would be
contrary to anything that is customary API design).
Also looking at the source there definitely is no need to preserve the
string after the call and there's no really reasonable way they would
change the source that would cause such a requirement.