On Thu, 2009-10-01 at 10:47 -0700, Jamey Sharp wrote: > --- > I didn't test my previous patch right. Sorry. This version doesn't seem > to crash the server at startup. :-) > > Review would still be greatly appreciated.
Thanks for jumping in! Comments inline.
> Xext/shm.c | 67 +++++++++++++++++++++++++++++++++++++++++++----------------
> 1 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/Xext/shm.c b/Xext/shm.c
> index a6f804c..b4167ac 100644
> --- a/Xext/shm.c
> +++ b/Xext/shm.c
> @@ -99,6 +99,11 @@ typedef struct _ShmDesc {
> unsigned long size;
> } ShmDescRec, *ShmDescPtr;
>
> +typedef struct _ShmScrPrivateRec {
> + ShmFuncsPtr shmFuncs;
> + DestroyPixmapProcPtr destroyPixmap;
> +} ShmScrPrivateRec;
> +
> static PixmapPtr fbShmCreatePixmap(XSHM_CREATE_PIXMAP_ARGS);
> static int ShmDetachSegment(
> pointer /* value */,
> @@ -135,13 +140,16 @@ int BadShmSegCode;
> RESTYPE ShmSegType;
> static ShmDescPtr Shmsegs;
> static Bool sharedPixmaps;
> -static ShmFuncsPtr shmFuncs[MAXSCREENS];
> -static DestroyPixmapProcPtr destroyPixmap[MAXSCREENS];
> +static DrawablePtr *drawables;
> +static int shmScrPrivateKeyIndex;
> +static DevPrivateKey shmScrPrivateKey = &shmScrPrivateKeyIndex;
> static int shmPixmapPrivateIndex;
> static DevPrivateKey shmPixmapPrivate = &shmPixmapPrivateIndex;
> static ShmFuncs miFuncs = {NULL, NULL};
> static ShmFuncs fbFuncs = {fbShmCreatePixmap, NULL};
>
> +#define ShmGetScreenPriv(s) ((ShmScrPrivateRec
> *)dixLookupPrivate(&(s)->devPrivates, shmScrPrivateKey))
> +
> #define VERIFY_SHMSEG(shmseg,shmdesc,client) \
> { \
> int rc; \
> @@ -212,6 +220,18 @@ static Bool CheckForShmSyscall(void)
>
> #endif
>
> +static ShmScrPrivateRec *
> +ShmInitScreenPriv(ScreenPtr pScreen)
> +{
> + ShmScrPrivateRec *priv = ShmGetScreenPriv(pScreen);
> + if (!priv)
> + {
> + priv = xcalloc (1, sizeof (ShmScrPrivateRec));
> + dixSetPrivate(&pScreen->devPrivates, shmScrPrivateKey, priv);
> + }
> + return priv;
> +}
> +
> void
> ShmExtensionInit(INITARGS)
> {
> @@ -226,20 +246,29 @@ ShmExtensionInit(INITARGS)
> }
> #endif
>
> + drawables = xcalloc(screenInfo.numScreens, sizeof(DrawablePtr));
> + if (!drawables)
> + {
> + ErrorF("MIT-SHM extension disabled: no memory for per-screen
> drawables\n");
> + return;
> + }
Seems like this drawable pointer should be part of the screen private.
> +
> sharedPixmaps = xFalse;
> {
> sharedPixmaps = xTrue;
> for (i = 0; i < screenInfo.numScreens; i++)
> {
> - if (!shmFuncs[i])
> - shmFuncs[i] = &miFuncs;
> - if (!shmFuncs[i]->CreatePixmap)
> + ShmScrPrivateRec *priv = ShmInitScreenPriv(screenInfo.screens[i]);
> + if (!priv->shmFuncs)
> + priv->shmFuncs = &miFuncs;
> + if (!priv->shmFuncs->CreatePixmap)
> sharedPixmaps = xFalse;
> }
> if (sharedPixmaps)
> for (i = 0; i < screenInfo.numScreens; i++)
> {
> - destroyPixmap[i] = screenInfo.screens[i]->DestroyPixmap;
> + ShmScrPrivateRec *priv = ShmGetScreenPriv(screenInfo.screens[i]);
> + priv->destroyPixmap = screenInfo.screens[i]->DestroyPixmap;
> screenInfo.screens[i]->DestroyPixmap = ShmDestroyPixmap;
> }
> }
> @@ -261,23 +290,21 @@ static void
> ShmResetProc(ExtensionEntry *extEntry)
> {
> int i;
> -
> - for (i = 0; i < MAXSCREENS; i++)
> - {
> - shmFuncs[i] = NULL;
> - }
> + for (i = 0; i < screenInfo.numScreens; i++)
> + ShmRegisterFuncs(screenInfo.screens[i], NULL);
> }
>
> void
> ShmRegisterFuncs(ScreenPtr pScreen, ShmFuncsPtr funcs)
> {
> - shmFuncs[pScreen->myNum] = funcs;
> + ShmInitScreenPriv(pScreen)->shmFuncs = funcs;
> }
I think this one might be a GetScreenPriv instead? I'm guessing that
ShmExtensionInit happens (set up protocol stuff), then later on DDXes
call ShmRegisterFuncs on their screen.
>
> static Bool
> ShmDestroyPixmap (PixmapPtr pPixmap)
> {
> ScreenPtr pScreen = pPixmap->drawable.pScreen;
> + ShmScrPrivateRec *priv;
> Bool ret;
> if (pPixmap->refcnt == 1)
> {
> @@ -288,9 +315,10 @@ ShmDestroyPixmap (PixmapPtr pPixmap)
> ShmDetachSegment ((pointer) shmdesc, pPixmap->drawable.id);
> }
>
> - pScreen->DestroyPixmap = destroyPixmap[pScreen->myNum];
> + priv = ShmGetScreenPriv(pScreen);
> + pScreen->DestroyPixmap = priv->destroyPixmap;
> ret = (*pScreen->DestroyPixmap) (pPixmap);
> - destroyPixmap[pScreen->myNum] = pScreen->DestroyPixmap;
> + priv->destroyPixmap = pScreen->DestroyPixmap;
> pScreen->DestroyPixmap = ShmDestroyPixmap;
> return ret;
> }
> @@ -298,7 +326,7 @@ ShmDestroyPixmap (PixmapPtr pPixmap)
> void
> ShmRegisterFbFuncs(ScreenPtr pScreen)
> {
> - shmFuncs[pScreen->myNum] = &fbFuncs;
> + ShmRegisterFuncs(pScreen, &fbFuncs);
> }
>
> static int
> @@ -578,7 +606,6 @@ static int
> ProcPanoramiXShmGetImage(ClientPtr client)
> {
> PanoramiXRes *draw;
> - DrawablePtr drawables[MAXSCREENS];
> DrawablePtr pDraw;
> xShmGetImageReply xgi;
> ShmDescPtr shmdesc;
> @@ -767,9 +794,11 @@ CreatePmap:
> result = (client->noClientException);
>
> FOR_NSCREENS(j) {
> + ShmScrPrivateRec *priv;
> pScreen = screenInfo.screens[j];
>
> - pMap = (*shmFuncs[j]->CreatePixmap)(pScreen,
> + priv = ShmGetScreenPriv(pScreen);
Stylistically, I like private fetching to be part of the declaration.
Less chance for early dereferencing. Also, as many layers have screen
privates, pixmap privates, gc privates, etc., it can be nice to name the
variables for the privates screen_priv, pixmap_priv, etc.
> + pMap = (*priv->shmFuncs->CreatePixmap)(pScreen,
> stuff->width, stuff->height, stuff->depth,
> shmdesc->addr + stuff->offset);
>
> @@ -1052,6 +1081,7 @@ ProcShmCreatePixmap(ClientPtr client)
> DepthPtr pDepth;
> int i, rc;
> ShmDescPtr shmdesc;
> + ShmScrPrivateRec *priv;
> REQUEST(xShmCreatePixmapReq);
> unsigned int width, height, depth;
> unsigned long size;
> @@ -1100,7 +1130,8 @@ CreatePmap:
> return BadAlloc;
>
> VERIFY_SHMSIZE(shmdesc, stuff->offset, size, client);
> - pMap = (*shmFuncs[pDraw->pScreen->myNum]->CreatePixmap)(
> + priv = ShmGetScreenPriv(pDraw->pScreen);
> + pMap = (*priv->shmFuncs->CreatePixmap)(
> pDraw->pScreen, stuff->width,
> stuff->height, stuff->depth,
> shmdesc->addr + stuff->offset);
--
Eric Anholt
e...@anholt.net eric.anh...@intel.com
signature.asc
Description: This is a digitally signed message part
_______________________________________________ xorg-devel mailing list xorg-devel@lists.x.org http://lists.x.org/mailman/listinfo/xorg-devel
