[PATCH] dix/glx/composite: consolidate visual resize in one place.

Dave Airlie airlied at redhat.com
Mon Sep 28 16:30:45 PDT 2009


The previous code was copied and in both cases incorrectly fixed
up the colormaps after resizing the visuals, this patch consolidates
the visual resize + colormaps fixups in one place.

I'm not 100% sure colormap.[ch] is the correct place for this but
visuals are mostly created in fb and I know thats not the place to
be resizing them.

Fixes fd.o bug #19470.

Signed-off-by: Dave Airlie <airlied at redhat.com>
---
 composite/compinit.c |   48 +++---------------------------------------------
 dix/colormap.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
 glx/glxscreens.c     |   48 +++++++-----------------------------------------
 include/colormap.h   |    4 ++++
 4 files changed, 57 insertions(+), 86 deletions(-)

diff --git a/composite/compinit.c b/composite/compinit.c
index 6159e4e..2d44166 100644
--- a/composite/compinit.c
+++ b/composite/compinit.c
@@ -249,11 +249,6 @@ 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,47 +276,10 @@ 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);
+    if (ResizeVisualArray(pScreen, 1) == FALSE)
+        return FALSE;
 
-    pScreen->visuals = visuals;
-    visual = visuals + pScreen->numVisuals; /* the new one */
-    pScreen->numVisuals++;
+    visual = visuals + (pScreen->numVisuals - 1); /* the new one */
 
     /* Initialize the visual */
     visual->vid = FakeClientID (0);
diff --git a/dix/colormap.c b/dix/colormap.c
index a5a006e..5183633 100644
--- a/dix/colormap.c
+++ b/dix/colormap.c
@@ -2690,3 +2690,46 @@ 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 */
+Bool
+ResizeVisualArray(ScreenPtr pScreen, int new_vis_count)
+{
+    struct colormap_lookup_data cdata;
+    int numVisuals;
+    VisualPtr visuals;
+
+    numVisuals = pScreen->numVisuals + new_vis_count;
+    visuals = xrealloc(pScreen->visuals, numVisuals * sizeof(VisualRec));
+    if (!visuals) {
+	return FALSE;
+    }
+
+    cdata.visuals = visuals;
+    cdata.pScreen = pScreen;
+    FindClientResourcesByType(serverClient, RT_COLORMAP, _colormap_find_resource, &cdata);
+
+    pScreen->visuals = visuals;
+    pScreen->numVisuals += new_vis_count;
+
+    return TRUE;
+}
diff --git a/glx/glxscreens.c b/glx/glxscreens.c
index 81faddd..2f41c92 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;
-    VisualPtr	 visuals;
-    ColormapPtr	 installedCmap;
+    XID		 *vids, vid;
+    int		 i;
     DepthPtr	 depth;
-    int		 rc;
+    int first_new_visual;
 
     depth = NULL;
     for (i = 0; i < pScreen->numDepths; i++) {
@@ -267,54 +265,22 @@ 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;
+    first_new_visual = pScreen->numVisuals;
 
-    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;
-    }
+    if (ResizeVisualArray(pScreen, count) == FALSE)
+        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);
-
     for (i = 0; i < count; i++) {
 	vid = FakeClientID(0);
-	visuals[pScreen->numVisuals + i].vid = vid;
+	pScreen->visuals[first_new_visual + i].vid = vid;
 	vids[depth->numVids + i] = vid;
     }
 
-    pScreen->visuals = visuals;
-    pScreen->numVisuals += count;
     depth->vids = vids;
     depth->numVids += count;
 
diff --git a/include/colormap.h b/include/colormap.h
index a3467c9..708b74d 100644
--- a/include/colormap.h
+++ b/include/colormap.h
@@ -179,4 +179,8 @@ extern _X_EXPORT int IsMapInstalled(
     Colormap /*map*/,
     WindowPtr /*pWin*/);
 
+extern _X_EXPORT Bool ResizeVisualArray(
+    ScreenPtr /* pScreen */, 
+    int /* new_vis_count */);
+
 #endif /* CMAP_H */
-- 
1.6.4.2



More information about the xorg-devel mailing list