From 43dd9e5f43ac5e0568e0b7af5cd79ce29bee5896 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Fri, 14 Mar 2025 13:54:42 +0100 Subject: [PATCH] dix: clean up MakeWindowOptional() calls and add alloc fault checks a) no need to checking for win->optional == NULL before calling MakeWindowOptional(), because it checks itself (except some cases where it's presence has it's own semantics, or prevent unnecessary allocations) b) lots of call sites didn't check for allocation failure. Signed-off-by: Enrico Weigelt, metux IT consult --- Xext/saver.c | 12 +++++------- Xext/shape.c | 20 ++++++++++++-------- Xi/exevents.c | 2 +- dix/events.c | 4 ++-- dix/grabs.c | 5 ++--- dix/property.c | 2 +- dix/window.c | 8 ++++---- xfixes/region.c | 4 ++-- 8 files changed, 29 insertions(+), 28 deletions(-) diff --git a/Xext/saver.c b/Xext/saver.c index e5204b932..4e5007a62 100644 --- a/Xext/saver.c +++ b/Xext/saver.c @@ -495,13 +495,11 @@ CreateSaverWindow(ScreenPtr pScreen) mask |= CWBorderPixmap; } if (pAttr->pCursor) { - CursorPtr cursor; - if (!pWin->optional) - if (!MakeWindowOptional(pWin)) { - FreeResource(pWin->drawable.id, X11_RESTYPE_NONE); - return FALSE; - } - cursor = RefCursor(pAttr->pCursor); + if (!MakeWindowOptional(pWin)) { + FreeResource(pWin->drawable.id, X11_RESTYPE_NONE); + return FALSE; + } + CursorPtr cursor = RefCursor(pAttr->pCursor); if (pWin->optional->cursor) FreeCursor(pWin->optional->cursor, (Cursor) 0); pWin->optional->cursor = cursor; diff --git a/Xext/shape.c b/Xext/shape.c index 1630a9ef3..ed99d7d8e 100644 --- a/Xext/shape.c +++ b/Xext/shape.c @@ -268,8 +268,9 @@ ShapeRectangles(ClientPtr client, xShapeRectanglesReq *stuff) return BadMatch; srcRgn = RegionFromRects(nrects, prects, ctype); - if (!pWin->optional) - MakeWindowOptional(pWin); + if (!MakeWindowOptional(pWin)) + return BadAlloc; + switch (stuff->destKind) { case ShapeBounding: destRgn = &pWin->optional->boundingShape; @@ -364,8 +365,9 @@ ShapeMask(ClientPtr client, xShapeMaskReq *stuff) return BadAlloc; } - if (!pWin->optional) - MakeWindowOptional(pWin); + if (!MakeWindowOptional(pWin)) + return BadAlloc; + switch (stuff->destKind) { case ShapeBounding: destRgn = &pWin->optional->boundingShape; @@ -441,8 +443,9 @@ ShapeCombine(ClientPtr client, xShapeCombineReq *stuff) rc = dixLookupWindow(&pDestWin, stuff->dest, client, DixSetAttrAccess); if (rc != Success) return rc; - if (!pDestWin->optional) - MakeWindowOptional(pDestWin); + if (!MakeWindowOptional(pDestWin)) + return BadAlloc; + switch (stuff->destKind) { case ShapeBounding: createDefault = CreateBoundingShape; @@ -490,8 +493,9 @@ ShapeCombine(ClientPtr client, xShapeCombineReq *stuff) else srcRgn = (*createSrc) (pSrcWin); - if (!pDestWin->optional) - MakeWindowOptional(pDestWin); + if (!MakeWindowOptional(pDestWin)) + return BadAlloc; + switch (stuff->destKind) { case ShapeBounding: destRgn = &pDestWin->optional->boundingShape; diff --git a/Xi/exevents.c b/Xi/exevents.c index 960d1381e..c82f9016e 100644 --- a/Xi/exevents.c +++ b/Xi/exevents.c @@ -2740,7 +2740,7 @@ AddExtensionClient(WindowPtr pWin, ClientPtr client, Mask mask, int mskidx) { InputClientsPtr others; - if (!pWin->optional && !MakeWindowOptional(pWin)) + if (!MakeWindowOptional(pWin)) return BadAlloc; others = AllocInputClient(); if (!others) diff --git a/dix/events.c b/dix/events.c index 3058efec7..0e2745b48 100644 --- a/dix/events.c +++ b/dix/events.c @@ -4587,7 +4587,7 @@ XRetCode EventSelectForWindow(WindowPtr pWin, ClientPtr client, Mask mask) } } check = 0; - if (!pWin->optional && !MakeWindowOptional(pWin)) + if (!MakeWindowOptional(pWin)) return BadAlloc; others = calloc(1, sizeof(OtherClients)); if (!others) @@ -4646,7 +4646,7 @@ EventSuppressForWindow(WindowPtr pWin, ClientPtr client, } } else { - if (!pWin->optional && !MakeWindowOptional(pWin)) { + if (!MakeWindowOptional(pWin)) { if (pWin->dontPropagate) DontPropagateRefCnts[pWin->dontPropagate]++; return BadAlloc; diff --git a/dix/grabs.c b/dix/grabs.c index fafd724e3..aabb0af4a 100644 --- a/dix/grabs.c +++ b/dix/grabs.c @@ -564,7 +564,7 @@ AddPassiveGrabToList(ClientPtr client, GrabPtr pGrab) } } - if (!pGrab->window->optional && !MakeWindowOptional(pGrab->window)) { + if (!MakeWindowOptional(pGrab->window)) { FreeGrab(pGrab); return BadAlloc; } @@ -663,8 +663,7 @@ DeletePassiveGrabFromList(GrabPtr pMinuendGrab) DeleteDetailFromMask(grab->modifiersDetail.pMask, pMinuendGrab->modifiersDetail. exact)) - || (!pNewGrab->window->optional && - !MakeWindowOptional(pNewGrab->window))) { + || (!MakeWindowOptional(pNewGrab->window))) { FreeGrab(pNewGrab); ok = FALSE; } diff --git a/dix/property.c b/dix/property.c index e0456711c..eb51e662d 100644 --- a/dix/property.c +++ b/dix/property.c @@ -316,7 +316,7 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property, rc = dixLookupProperty(&pProp, pWin, property, pClient, access_mode); if (rc == BadMatch) { /* just add to list */ - if (!pWin->optional && !MakeWindowOptional(pWin)) + if (!MakeWindowOptional(pWin)) return BadAlloc; pProp = dixAllocateObjectWithPrivates(PropertyRec, PRIVATE_PROPERTY); if (!pProp) diff --git a/dix/window.c b/dix/window.c index 013fc5ceb..7c3e157c4 100644 --- a/dix/window.c +++ b/dix/window.c @@ -1317,7 +1317,7 @@ ChangeWindowAttributes(WindowPtr pWin, Mask vmask, XID *vlist, ClientPtr client) break; case CWBackingPlanes: if (pWin->optional || ((CARD32) *pVlist != (CARD32) ~0L)) { - if (!pWin->optional && !MakeWindowOptional(pWin)) { + if (!MakeWindowOptional(pWin)) { error = BadAlloc; goto PatchUp; } @@ -1329,7 +1329,7 @@ ChangeWindowAttributes(WindowPtr pWin, Mask vmask, XID *vlist, ClientPtr client) break; case CWBackingPixel: if (pWin->optional || (CARD32) *pVlist) { - if (!pWin->optional && !MakeWindowOptional(pWin)) { + if (!MakeWindowOptional(pWin)) { error = BadAlloc; goto PatchUp; } @@ -1430,7 +1430,7 @@ ChangeWindowAttributes(WindowPtr pWin, Mask vmask, XID *vlist, ClientPtr client) for (pChild = pWin->firstChild; pChild; pChild = pChild->nextSib) { - if (!pChild->optional && !MakeWindowOptional(pChild)) { + if (!MakeWindowOptional(pChild)) { error = BadAlloc; goto PatchUp; } @@ -3430,7 +3430,7 @@ ChangeWindowDeviceCursor(WindowPtr pWin, DeviceIntPtr pDev, CursorPtr pCursor) ScreenPtr pScreen; WindowPtr pChild; - if (!pWin->optional && !MakeWindowOptional(pWin)) + if (!MakeWindowOptional(pWin)) return BadAlloc; /* 1) Check if window has device cursor set diff --git a/xfixes/region.c b/xfixes/region.c index 0813ae1f4..d4458afc2 100644 --- a/xfixes/region.c +++ b/xfixes/region.c @@ -658,8 +658,8 @@ SingleXFixesSetWindowShapeRegion(ClientPtr client, xXFixesSetWindowShapeRegionRe pRegion = XFixesRegionCopy(pRegion); if (!pRegion) return BadAlloc; - if (!pWin->optional) - MakeWindowOptional(pWin); + if (!MakeWindowOptional(pWin)) + return BadAlloc; switch (stuff->destKind) { default: case ShapeBounding: