[Mesa-dev] [PATCH] xlib: do not cache return value of glXChooseVisual/glXGetVisualFromFBConfig

[Alejandro Piñeiro]

It would be good to mention that XMesaVisual.vishandle would be removed
on an following patch, and also it would be good to add a v2 comment.
With those nitpicks:

Reviewed-by: Alejandro Piñeiro <apinheiro at igalia.com>

On 13/04/16 01:08, John Sheu wrote:
> The returned XVisualInfo from glXChooseVisual/glXGetVisualFromFBConfig
> is being cached in XMesaVisual.vishandle (and unconditionally
> overwritten on subsequent calls).  However, these entry points are
> specified to return XVisualInfo instances to be owned by the caller and
> freed with XFree(), so the return values should not be retained.
> ---
>  src/mesa/drivers/x11/fakeglx.c | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/src/mesa/drivers/x11/fakeglx.c b/src/mesa/drivers/x11/fakeglx.c
> index 2f4d9669..d62d5abd 100644
> --- a/src/mesa/drivers/x11/fakeglx.c
> +++ b/src/mesa/drivers/x11/fakeglx.c
> @@ -1241,16 +1241,11 @@ Fake_glXChooseVisual( Display *dpy, int screen, int *list )
>  
>     xmvis = choose_visual(dpy, screen, list, GL_FALSE);
>     if (xmvis) {
> -#if 0
> -      return xmvis->vishandle;
> -#else
> -      /* create a new vishandle - the cached one may be stale */
> -      xmvis->vishandle = malloc(sizeof(XVisualInfo));
> -      if (xmvis->vishandle) {
> -         memcpy(xmvis->vishandle, xmvis->visinfo, sizeof(XVisualInfo));
> +      XVisualInfo* visinfo = malloc(sizeof(XVisualInfo));
> +      if (visinfo) {
> +         memcpy(visinfo, xmvis->visinfo, sizeof(XVisualInfo));
>        }
> -      return xmvis->vishandle;
> -#endif
> +      return visinfo;
>     }
>     else
>        return NULL;
> @@ -1974,16 +1969,11 @@ Fake_glXGetVisualFromFBConfig( Display *dpy, GLXFBConfig config )
>  {
>     if (dpy && config) {
>        XMesaVisual xmvis = (XMesaVisual) config;
> -#if 0      
> -      return xmvis->vishandle;
> -#else
> -      /* create a new vishandle - the cached one may be stale */
> -      xmvis->vishandle = malloc(sizeof(XVisualInfo));
> -      if (xmvis->vishandle) {
> -         memcpy(xmvis->vishandle, xmvis->visinfo, sizeof(XVisualInfo));
> +      XVisualInfo* visinfo = malloc(sizeof(XVisualInfo));
> +      if (visinfo) {
> +         memcpy(visinfo, xmvis->visinfo, sizeof(XVisualInfo));
>        }
> -      return xmvis->vishandle;
> -#endif
> +      return visinfo;
>     }
>     else {
>        return NULL;