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

[Alejandro Piñeiro]

For next time, on the cases were minor changes are suggested, but a RB
is granted in any case, you don't need to send the patch again to the
list, you can made the changes locally and push directly without
re-submitting to the list.

BTW, talking about pushing, do you have push permissions? I can do the
push of this and the other two patches if needed.

BR

On 13/04/16 22:57, 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.
>
> With this change, XMesaVisual.vishandle is essentially unused and will
> be removed in a subsequent change.
> ---
>  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;