[Mesa-dev] [PATCH 2/4] xlib: fix memory leak of and remove vishandle from XMesaVisualInfo

[John Sheu]

The vishandle member of XMesaVisualInfo is used to support the
comparison of XVisualInfo instances by pointer value, in
find_glx_visual().  The comparison however will always be false, as in
every case the comparison is made, the VisualInfo instance being
compared to is a new allocation passed in through a GLX API call.

In addition, the XVisualInfo instance pointed to by vishandle is itself
never freed, causing a memory leak.  Since vishandle is essentially
useless, we just remove it and thereby also fix the leak.
---
 src/mesa/drivers/x11/fakeglx.c | 62 ++++++++++++++++--------------------------
 src/mesa/drivers/x11/xmesaP.h  |  1 -
 2 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/src/mesa/drivers/x11/fakeglx.c b/src/mesa/drivers/x11/fakeglx.c
index d62d5abd..208fc5bb 100644
--- a/src/mesa/drivers/x11/fakeglx.c
+++ b/src/mesa/drivers/x11/fakeglx.c
@@ -256,7 +256,6 @@ save_glx_visual( Display *dpy, XVisualInfo *vinfo,
    GLboolean ximageFlag = GL_TRUE;
    XMesaVisual xmvis;
    GLint i;
-   GLboolean comparePointers;
 
    if (dbFlag) {
       /* Check if the MESA_BACK_BUFFER env var is set */
@@ -279,37 +278,34 @@ save_glx_visual( Display *dpy, XVisualInfo *vinfo,
       return NULL;
    }
 
-   /* Comparing IDs uses less memory but sometimes fails. */
-   /* XXX revisit this after 3.0 is finished. */
-   if (getenv("MESA_GLX_VISUAL_HACK"))
-      comparePointers = GL_TRUE;
-   else
-      comparePointers = GL_FALSE;
 
    /* Force the visual to have an alpha channel */
    if (getenv("MESA_GLX_FORCE_ALPHA"))
       alphaFlag = GL_TRUE;
 
-   /* First check if a matching visual is already in the list */
-   for (i=0; i<NumVisuals; i++) {
-      XMesaVisual v = VisualTable[i];
-      if (v->display == dpy
-          && v->mesa_visual.level == level
-          && v->mesa_visual.numAuxBuffers == numAuxBuffers
-          && v->ximage_flag == ximageFlag
-          && v->mesa_visual.doubleBufferMode == dbFlag
-          && v->mesa_visual.stereoMode == stereoFlag
-          && (v->mesa_visual.alphaBits > 0) == alphaFlag
-          && (v->mesa_visual.depthBits >= depth_size || depth_size == 0)
-          && (v->mesa_visual.stencilBits >= stencil_size || stencil_size == 0)
-          && (v->mesa_visual.accumRedBits >= accumRedSize || accumRedSize == 0)
-          && (v->mesa_visual.accumGreenBits >= accumGreenSize || accumGreenSize == 0)
-          && (v->mesa_visual.accumBlueBits >= accumBlueSize || accumBlueSize == 0)
-          && (v->mesa_visual.accumAlphaBits >= accumAlphaSize || accumAlphaSize == 0)) {
-         /* now either compare XVisualInfo pointers or visual IDs */
-         if ((!comparePointers && v->visinfo->visualid == vinfo->visualid)
-             || (comparePointers && v->vishandle == vinfo)) {
-            return v;
+   /* Comparing IDs uses less memory but sometimes fails. */
+   /* XXX revisit this after 3.0 is finished. */
+   if (!getenv("MESA_GLX_VISUAL_HACK")) {
+      /* First check if a matching visual is already in the list */
+      for (i=0; i<NumVisuals; i++) {
+         XMesaVisual v = VisualTable[i];
+         if (v->display == dpy
+             && v->mesa_visual.level == level
+             && v->mesa_visual.numAuxBuffers == numAuxBuffers
+             && v->ximage_flag == ximageFlag
+             && v->mesa_visual.doubleBufferMode == dbFlag
+             && v->mesa_visual.stereoMode == stereoFlag
+             && (v->mesa_visual.alphaBits > 0) == alphaFlag
+             && (v->mesa_visual.depthBits >= depth_size || depth_size == 0)
+             && (v->mesa_visual.stencilBits >= stencil_size || stencil_size == 0)
+             && (v->mesa_visual.accumRedBits >= accumRedSize || accumRedSize == 0)
+             && (v->mesa_visual.accumGreenBits >= accumGreenSize || accumGreenSize == 0)
+             && (v->mesa_visual.accumBlueBits >= accumBlueSize || accumBlueSize == 0)
+             && (v->mesa_visual.accumAlphaBits >= accumAlphaSize || accumAlphaSize == 0)) {
+            /* now compare visual IDs */
+            if (v->visinfo->visualid == vinfo->visualid) {
+               return v;
+            }
          }
       }
    }
@@ -323,10 +319,6 @@ save_glx_visual( Display *dpy, XVisualInfo *vinfo,
                               accumBlueSize, accumAlphaSize, 0, level,
                               GLX_NONE_EXT );
    if (xmvis) {
-      /* Save a copy of the pointer now so we can find this visual again
-       * if we need to search for it in find_glx_visual().
-       */
-      xmvis->vishandle = vinfo;
       /* Allocate more space for additional visual */
       VisualTable = realloc(VisualTable, sizeof(XMesaVisual) * (NumVisuals + 1));
       /* add xmvis to the list */
@@ -442,13 +434,6 @@ find_glx_visual( Display *dpy, XVisualInfo *vinfo )
       }
    }
 
-   /* if that fails, try to match pointers */
-   for (i=0;i<NumVisuals;i++) {
-      if (VisualTable[i]->display==dpy && VisualTable[i]->vishandle==vinfo) {
-         return VisualTable[i];
-      }
-   }
-
    return NULL;
 }
 
@@ -1225,6 +1210,7 @@ choose_visual( Display *dpy, int screen, const int *list, GLboolean fbConfig )
                                stereo_flag, depth_size, stencil_size,
                                accumRedSize, accumGreenSize,
                                accumBlueSize, accumAlphaSize, level, numAux );
+      free(vis);
    }
 
    return xmvis;
diff --git a/src/mesa/drivers/x11/xmesaP.h b/src/mesa/drivers/x11/xmesaP.h
index d7a934e8..6cd020f2 100644
--- a/src/mesa/drivers/x11/xmesaP.h
+++ b/src/mesa/drivers/x11/xmesaP.h
@@ -78,7 +78,6 @@ struct xmesa_visual {
    int screen, visualID;
    int visualType;
    XMesaVisualInfo visinfo;	/* X's visual info (pointer to private copy) */
-   XVisualInfo *vishandle;	/* Only used in fakeglx.c */
    GLint BitsPerPixel;		/* True bits per pixel for XImages */
 
    GLboolean ximage_flag;	/* Use XImage for back buffer (not pixmap)? */
-- 
2.8.0.rc3.226.g39d4020