From 781232e0ac48bf608757bbd270c593a90173f951 Mon Sep 17 00:00:00 2001 From: John Sheu Date: Fri, 1 Apr 2016 16:52:20 -0700 Subject: [PATCH] xlib: fix memory leak of and remove vishandle from XMesaVisualInfo MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Alejandro Piñeiro --- 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 d62d5abd465..208fc5bbc60 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; idisplay == 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; idisplay == 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;idisplay==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 d7a934e8df4..6cd020f2ed3 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.30.2