Advantages: - Spares callers from allocating a MAXSCREENS-sized temporary array. - Hides implementation details of the PanoramiXRes info array. - Avoids looking up drawables on screens that are entirely clipped out of the image bounds.
Disadvantages: When called in a loop, may look up the same drawable multiple times. This only affects XYPixmaps and images larger than 256kB, so it should have minimal real impact. Validates Xinerama-allocated XIDs late, so if the Xinerama wrapper didn't initialize the PanoramiXRes info array correctly, that error can't always be reported correctly. It seems to me that if we have a valid PanoramiXRes that this client is allowed access to, we ought to be able to get all the per-screen drawables that go with it too, so I don't think this error case should be possible. Signed-off-by: Jamey Sharp <[email protected]> --- Have I analyzed this correctly? And especially, can I assume that per-screen drawables are accessible if the PanoramiXRes is? If so, perhaps errors should just be ignored here? Xext/panoramiX.c | 28 ++++++++++++++++++++-------- Xext/panoramiXprocs.c | 38 ++++++++++++++++---------------------- Xext/panoramiXsrv.h | 6 ++++-- Xext/shm.c | 31 ++++++++++--------------------- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/Xext/panoramiX.c b/Xext/panoramiX.c index 96eb8f9..50fa970 100644 --- a/Xext/panoramiX.c +++ b/Xext/panoramiX.c @@ -1135,9 +1135,11 @@ CopyBits(char *dst, int shiftL, char *src, int bytes) 1 bpp and planar data to be already cleared when presented to this function */ -void +int XineramaGetImageData( - DrawablePtr *pDrawables, + PanoramiXRes *xr_drawable, + ClientPtr client, + Mask access_mode, int left, int top, int width, @@ -1150,10 +1152,14 @@ XineramaGetImageData( ){ RegionRec SrcRegion, GrabRegion; BoxRec SrcBox, *pbox; - int x, y, w, h, i, j, nbox, size, sizeNeeded, ScratchPitch, inOut, depth; - DrawablePtr pDraw = pDrawables[0]; + int x, y, w, h, i, j, nbox, size, sizeNeeded, ScratchPitch, inOut, depth, rc; + DrawablePtr pDraw; char *ScratchMem = NULL; + rc = dixLookupDrawable(&pDraw, xr_drawable->info[0].id, client, 0, access_mode); + if (rc != Success) + return rc; + size = 0; /* find box in logical screen space */ @@ -1172,9 +1178,15 @@ XineramaGetImageData( depth = (format == XYPixmap) ? 1 : pDraw->depth; for(i = 0; i < PanoramiXNumScreens; i++) { - pDraw = pDrawables[i]; - inOut = RECT_IN_REGION(pScreen,&XineramaScreenRegions[i],&SrcBox); + if (inOut == rgnOUT) + continue; + + if (i != 0) { /* already found first drawable outside the loop */ + rc = dixLookupDrawable(&pDraw, xr_drawable->info[i].id, client, 0, access_mode); + if (rc != Success) + break; + } if(inOut == rgnIN) { (*pDraw->pScreen->GetImage)(pDraw, @@ -1182,8 +1194,7 @@ XineramaGetImageData( SrcBox.y1 - pDraw->y - panoramiXdataPtr[i].y, width, height, format, planemask, data); break; - } else if (inOut == rgnOUT) - continue; + } REGION_INTERSECT(pScreen, &GrabRegion, &SrcRegion, &XineramaScreenRegions[i]); @@ -1280,4 +1291,5 @@ XineramaGetImageData( REGION_UNINIT(pScreen, &SrcRegion); REGION_UNINIT(pScreen, &GrabRegion); + return rc; } diff --git a/Xext/panoramiXprocs.c b/Xext/panoramiXprocs.c index 6834efb..3baa8f9 100644 --- a/Xext/panoramiXprocs.c +++ b/Xext/panoramiXprocs.c @@ -1050,31 +1050,30 @@ int PanoramiXCopyArea(ClientPtr client) srcx = stuff->srcX; srcy = stuff->srcY; dstx = stuff->dstX; dsty = stuff->dstY; if((dst->type == XRT_PIXMAP) && (src->type == XRT_WINDOW)) { - DrawablePtr drawables[MAXSCREENS]; - DrawablePtr pDst; + DrawablePtr pSrc, pDst; GCPtr pGC; char *data; int pitch, rc; - FOR_NSCREENS(j) { - rc = dixLookupDrawable(drawables+j, src->info[j].id, client, 0, + rc = dixLookupDrawable(&pSrc, src->info[0].id, client, 0, DixGetAttrAccess); - if (rc != Success) - return rc; - } + if (rc != Success) + return rc; - pitch = PixmapBytePad(stuff->width, drawables[0]->depth); + pitch = PixmapBytePad(stuff->width, pSrc->depth); if(!(data = xcalloc(1, stuff->height * pitch))) return BadAlloc; - XineramaGetImageData(drawables, srcx, srcy, + rc = XineramaGetImageData(src, client, DixGetAttrAccess, srcx, srcy, stuff->width, stuff->height, ZPixmap, ~0, data, pitch, srcIsRoot); + if (rc != Success) + return rc; FOR_NSCREENS_BACKWARD(j) { stuff->gc = gc->info[j].id; VALIDATE_DRAWABLE_AND_GC(dst->info[j].id, pDst, DixWriteAccess); - if(drawables[0]->depth != pDst->depth) { + if(pSrc->depth != pDst->depth) { client->errorValue = stuff->dstDrawable; xfree(data); return (BadMatch); @@ -1805,13 +1804,12 @@ int PanoramiXPutImage(ClientPtr client) int PanoramiXGetImage(ClientPtr client) { - DrawablePtr drawables[MAXSCREENS]; DrawablePtr pDraw; PanoramiXRes *draw; xGetImageReply xgi; Bool isRoot; char *pBuf; - int i, x, y, w, h, format, rc; + int x, y, w, h, format, rc; Mask plane = 0, planemask; int linesDone, nlines, linesPerBuf; long widthBytesLine, length; @@ -1869,14 +1867,6 @@ int PanoramiXGetImage(ClientPtr client) return(BadMatch); } - drawables[0] = pDraw; - for(i = 1; i < PanoramiXNumScreens; i++) { - rc = dixLookupDrawable(drawables+i, draw->info[i].id, client, 0, - DixGetAttrAccess); - if (rc != Success) - return rc; - } - xgi.visual = wVisual (((WindowPtr) pDraw)); xgi.type = X_Reply; xgi.sequenceNumber = client->sequence; @@ -1923,8 +1913,10 @@ int PanoramiXGetImage(ClientPtr client) if(pDraw->depth == 1) bzero(pBuf, nlines * widthBytesLine); - XineramaGetImageData(drawables, x, y + linesDone, w, nlines, + rc = XineramaGetImageData(draw, client, DixGetAttrAccess, x, y + linesDone, w, nlines, format, planemask, pBuf, widthBytesLine, isRoot); + if (rc != Success) + ErrorF("tried to read from unknown Xinerama drawables\n"); (void)WriteToClient(client, (int)(nlines * widthBytesLine), @@ -1940,9 +1932,11 @@ int PanoramiXGetImage(ClientPtr client) bzero(pBuf, nlines * widthBytesLine); - XineramaGetImageData(drawables, x, y + linesDone, w, + rc = XineramaGetImageData(draw, client, DixGetAttrAccess, x, y + linesDone, w, nlines, format, plane, pBuf, widthBytesLine, isRoot); + if (rc != Success) + ErrorF("tried to read from unknown Xinerama drawables\n"); (void)WriteToClient(client, (int)(nlines * widthBytesLine), diff --git a/Xext/panoramiXsrv.h b/Xext/panoramiXsrv.h index c77b119..f4e0c61 100644 --- a/Xext/panoramiXsrv.h +++ b/Xext/panoramiXsrv.h @@ -40,8 +40,10 @@ extern _X_EXPORT unsigned long XRT_COLORMAP; typedef Bool (*XineramaVisualsEqualProcPtr)(VisualPtr, ScreenPtr, VisualPtr); extern _X_EXPORT XineramaVisualsEqualProcPtr XineramaVisualsEqualPtr; -extern _X_EXPORT void XineramaGetImageData( - DrawablePtr *pDrawables, +extern _X_EXPORT int XineramaGetImageData( + PanoramiXRes *xr_drawable, + ClientPtr client, + Mask access_mode, int left, int top, int width, diff --git a/Xext/shm.c b/Xext/shm.c index ab58c27..7fc6675 100644 --- a/Xext/shm.c +++ b/Xext/shm.c @@ -610,11 +610,10 @@ static int ProcPanoramiXShmGetImage(ClientPtr client) { PanoramiXRes *draw; - DrawablePtr *drawables; DrawablePtr pDraw; xShmGetImageReply xgi; ShmDescPtr shmdesc; - int i, x, y, w, h, format, rc; + int x, y, w, h, format, rc; Mask plane = 0, planemask; long lenPer = 0, length, widthBytesLine; Bool isRoot; @@ -671,21 +670,6 @@ ProcPanoramiXShmGetImage(ClientPtr client) return(BadMatch); } - drawables = xcalloc(PanoramiXNumScreens, sizeof(DrawablePtr)); - if(!drawables) - return(BadAlloc); - - drawables[0] = pDraw; - for(i = 1; i < PanoramiXNumScreens; i++) { - rc = dixLookupDrawable(drawables+i, draw->info[i].id, client, 0, - DixReadAccess); - if (rc != Success) - { - xfree(drawables); - return rc; - } - } - xgi.visual = wVisual(((WindowPtr)pDraw)); xgi.type = X_Reply; xgi.length = 0; @@ -707,22 +691,27 @@ ProcPanoramiXShmGetImage(ClientPtr client) if (length == 0) {/* nothing to do */ } else if (format == ZPixmap) { - XineramaGetImageData(drawables, x, y, w, h, format, planemask, + rc = XineramaGetImageData(draw, client, DixReadAccess, + x, y, w, h, format, planemask, shmdesc->addr + stuff->offset, widthBytesLine, isRoot); + if (rc != Success) + return rc; } else { length = stuff->offset; for (; plane; plane >>= 1) { if (planemask & plane) { - XineramaGetImageData(drawables, x, y, w, h, - format, plane, shmdesc->addr + length, + rc = XineramaGetImageData(draw, client, DixReadAccess, + x, y, w, h, format, plane, + shmdesc->addr + length, widthBytesLine, isRoot); + if (rc != Success) + return rc; length += lenPer; } } } - xfree(drawables); if (client->swapped) { int n; -- 1.7.0 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
