[PATCH] dix/glx/composite: fixup colormap visuals correctly.

[Dave Airlie]

One of the first accurate comments I've come across:
/*
 * Fix up any existing installed colormaps -- we'll assume that
 * the only ones created so far have been installed.  If this
 * isn't true, we'll have to walk the resource database looking
 * for all colormaps.
 */
Guess what, it was right, installed colormaps are useless information
due to render installing a bunch, this meant every so often we'd have
a colormap pointing at a visual that wasn't there anymore since the
realloc moved it, then we'd free the colormap on server exit and
explode.

This patch does it right, it walks the resource list for colormaps
and re-writes the visual bits in them.

Fixes fd.o bug #19470.

Signed-off-by: Dave Airlie <airlied at redhat.com>
---
 composite/compinit.c |   34 +---------------------------------
 dix/colormap.c       |   29 +++++++++++++++++++++++++++++
 glx/glxscreens.c     |   33 +++------------------------------
 include/colormap.h   |    3 +++
 4 files changed, 36 insertions(+), 63 deletions(-)

diff --git a/composite/compinit.c b/composite/compinit.c
index 6159e4e..daf01d3 100644
--- a/composite/compinit.c
+++ b/composite/compinit.c
@@ -249,11 +249,7 @@ compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
 		       CompAlternateVisual *alt)
 {
     VisualPtr	    visual, visuals;
-    int		    i;
     int		    numVisuals;
-    XID		    *installedCmaps;
-    ColormapPtr	    installedCmap;
-    int		    numInstalledCmaps;
     DepthPtr	    depth;
     PictFormatPtr   pPictFormat;
     VisualID	    *vid;
@@ -281,43 +277,15 @@ compAddAlternateVisual(ScreenPtr pScreen, CompScreenPtr cs,
     if (!vid)
 	return FALSE;
 
-    /* Find the installed colormaps */
-    installedCmaps = xalloc (pScreen->maxInstalledCmaps * sizeof (XID));
-    if (!installedCmaps) {
-	xfree(vid);
-	return FALSE;
-    }
-    numInstalledCmaps = pScreen->ListInstalledColormaps(pScreen, 
-	    installedCmaps);
-
     /* realloc the visual array to fit the new one in place */
     numVisuals = pScreen->numVisuals;
     visuals = xrealloc(pScreen->visuals, (numVisuals + 1) * sizeof(VisualRec));
     if (!visuals) {
 	xfree(vid);
-	xfree(installedCmaps);
 	return FALSE;
     }
 
-    /*
-     * Fix up any existing installed colormaps -- we'll assume that
-     * the only ones created so far have been installed.  If this
-     * isn't true, we'll have to walk the resource database looking
-     * for all colormaps.
-     */
-    for (i = 0; i < numInstalledCmaps; i++) {
-	int j, rc;
-
-	rc = dixLookupResourceByType((pointer *)&installedCmap,
-				     installedCmaps[i], RT_COLORMAP,
-				     serverClient, DixReadAccess);
-	if (rc != Success)
-	    continue;
-	j = installedCmap->pVisual - pScreen->visuals;
-	installedCmap->pVisual = &visuals[j];
-    }
-
-    xfree(installedCmaps);
+    ColormapFixVisuals(pScreen, visuals);
 
     pScreen->visuals = visuals;
     visual = visuals + pScreen->numVisuals; /* the new one */
diff --git a/dix/colormap.c b/dix/colormap.c
index a5a006e..771fefc 100644
--- a/dix/colormap.c
+++ b/dix/colormap.c
@@ -2690,3 +2690,32 @@ IsMapInstalled(Colormap map, WindowPtr pWin)
     xfree(pmaps);
     return (found);
 }
+
+struct colormap_lookup_data {
+    ScreenPtr pScreen;
+    VisualPtr visuals;
+};
+
+static void _colormap_find_resource(pointer value, XID id,
+				    pointer cdata)
+{
+    struct colormap_lookup_data *cmap_data = cdata;
+    VisualPtr visuals = cmap_data->visuals;
+    ScreenPtr pScreen = cmap_data->pScreen;
+    ColormapPtr cmap = value;
+    int j;
+    
+    j = cmap->pVisual - pScreen->visuals;
+    cmap->pVisual = &visuals[j];
+}
+
+/* something has realloced the visuals, instead of breaking
+   ABI fix it up here - glx and compsite did this wrong */
+void
+ColormapFixVisuals(ScreenPtr pScreen, VisualPtr visuals)
+{
+    struct colormap_lookup_data cdata;
+    cdata.visuals = visuals;
+    cdata.pScreen = pScreen;
+    FindClientResourcesByType(serverClient, RT_COLORMAP, _colormap_find_resource, &cdata);
+}
diff --git a/glx/glxscreens.c b/glx/glxscreens.c
index 81faddd..f93acde 100644
--- a/glx/glxscreens.c
+++ b/glx/glxscreens.c
@@ -250,12 +250,10 @@ GLint glxConvertToXVisualType(int visualType)
 static VisualPtr
 AddScreenVisuals(ScreenPtr pScreen, int count, int d)
 {
-    XID		*installedCmaps, *vids, vid;
-    int		 numInstalledCmaps, numVisuals, i, j;
+    XID		 *vids, vid;
+    int		 numVisuals, i;
     VisualPtr	 visuals;
-    ColormapPtr	 installedCmap;
     DepthPtr	 depth;
-    int		 rc;
 
     depth = NULL;
     for (i = 0; i < pScreen->numDepths; i++) {
@@ -267,45 +265,20 @@ AddScreenVisuals(ScreenPtr pScreen, int count, int d)
     if (depth == NULL)
 	return NULL;
 
-    /* Find the installed colormaps */
-    installedCmaps = xalloc (pScreen->maxInstalledCmaps * sizeof (XID));
-    if (!installedCmaps)
-	return NULL;
-
-    numInstalledCmaps = pScreen->ListInstalledColormaps(pScreen, installedCmaps);
-
     /* realloc the visual array to fit the new one in place */
     numVisuals = pScreen->numVisuals;
     visuals = xrealloc(pScreen->visuals, (numVisuals + count) * sizeof(VisualRec));
     if (!visuals) {
-	xfree(installedCmaps);
 	return NULL;
     }
 
     vids = xrealloc(depth->vids, (depth->numVids + count) * sizeof(XID));
     if (vids == NULL) {
-	xfree(installedCmaps);
 	xfree(visuals);
 	return NULL;
     }
 
-    /*
-     * Fix up any existing installed colormaps -- we'll assume that
-     * the only ones created so far have been installed.  If this
-     * isn't true, we'll have to walk the resource database looking
-     * for all colormaps.
-     */
-    for (i = 0; i < numInstalledCmaps; i++) {
-	rc = dixLookupResourceByType((pointer *)&installedCmap,
-				     installedCmaps[i], RT_COLORMAP,
-				     serverClient, DixReadAccess);
-	if (rc != Success)
-	    continue;
-	j = installedCmap->pVisual - pScreen->visuals;
-	installedCmap->pVisual = &visuals[j];
-    }
-
-    xfree(installedCmaps);
+    ColormapFixVisuals(pScreen, visuals);
 
     for (i = 0; i < count; i++) {
 	vid = FakeClientID(0);
diff --git a/include/colormap.h b/include/colormap.h
index a3467c9..88585ec 100644
--- a/include/colormap.h
+++ b/include/colormap.h
@@ -179,4 +179,7 @@ extern _X_EXPORT int IsMapInstalled(
     Colormap /*map*/,
     WindowPtr /*pWin*/);
 
+extern _X_EXPORT void ColormapFixVisuals(
+    ScreenPtr /*pScreen*/, 
+    VisualPtr /*visuals*/);
 #endif /* CMAP_H */
-- 
1.6.4.2