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 <[email protected]> --- 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 _______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
