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

[Sven C. Dack]

On 01/04/11 16:56, Carl Eugen Hoyos wrote:
>       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?
>    
Yes. The way you have done it (with av_free() directly after 
XOpenDisplay()) expects that the memory never gets touched after the 
call to XOpenDisplay(). You cannot rely on such a behaviour without 
knowing the exact implementation details of all the X11 libraries on 
every available platform.

For example, it is likely dpy->display_name points to this memory area. 
It does not have to and may contain a copy of it, but if it does then 
its content can become invalid. Any code relying on a valid 
dpy->display_name can break.

It is better to respect the API by passing it a writeable string 
(because it expects a char*) and then to preserve the memory until the 
display is closed again.

Sven

PS: Feel free to take my patch and merge it with your own code.
> Carl Eugen
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>