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

[John Sheu]

Thanks!  I don't have push permissions actually, so all my stuff is going
through the mailing list at the moment.

-John Sheu

On Thu, Apr 14, 2016 at 12:09 AM Alejandro Piñeiro <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;
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160414/871f0506/attachment-0001.html>