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

[Alejandro Piñeiro]

On 14/04/16 19:10, John Sheu wrote:
> Thanks!  I don't have push permissions actually, so all my stuff is
> going through the mailing list at the moment.
>
> -John Sheu

Those 4 patches pushed.

Best regards.

>
> On Thu, Apr 14, 2016 at 12:09 AM Alejandro Piñeiro
> <apinheiro at igalia.com <mailto:apinheiro at igalia.com>> wrote:
>
>     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;
>