From 28b7bdf84a2d5d27bab3bb480ece081e6a8c6984 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 | 21 +++++++++++++-------- Xi/exevents.c | 3 ++- dix/events.c | 5 +++-- dix/grabs.c | 6 +++--- dix/property.c | 3 ++- dix/window.c | 8 ++++---- xfixes/region.c | 5 +++-- 8 files changed, 35 insertions(+), 28 deletions(-) diff --git a/Xext/saver.c b/Xext/saver.c index cb77399af..4f22d08cc 100644 --- a/Xext/saver.c +++ b/Xext/saver.c @@ -498,13 +498,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 7a0634ae2..adba52318 100644 --- a/Xext/shape.c +++ b/Xext/shape.c @@ -33,6 +33,7 @@ in this Software without prior written authorization from The Open Group. #include "dix/dix_priv.h" #include "dix/gc_priv.h" +#include "dix/window_priv.h" #include "misc.h" #include "os.h" @@ -270,8 +271,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; @@ -366,8 +368,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; @@ -443,8 +446,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; @@ -492,8 +496,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 753172c0e..20b1aa716 100644 --- a/Xi/exevents.c +++ b/Xi/exevents.c @@ -96,6 +96,7 @@ SOFTWARE. #include "dix/eventconvert.h" #include "dix/exevents_priv.h" #include "dix/input_priv.h" +#include "dix/window_priv.h" #include "mi/mi_priv.h" #include "inputstr.h" @@ -2733,7 +2734,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 7893b1413..eed329df0 100644 --- a/dix/events.c +++ b/dix/events.c @@ -126,6 +126,7 @@ Equipment Corporation. #include "dix/extension_priv.h" #include "dix/input_priv.h" #include "dix/reqhandlers_priv.h" +#include "dix/window_priv.h" #include "os/bug_priv.h" #include "os/client_priv.h" #include "os/fmt.h" @@ -4588,7 +4589,7 @@ EventSelectForWindow(WindowPtr pWin, ClientPtr client, Mask mask) } } check = 0; - if (!pWin->optional && !MakeWindowOptional(pWin)) + if (!MakeWindowOptional(pWin)) return BadAlloc; others = malloc(sizeof(OtherClients)); if (!others) @@ -4647,7 +4648,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 8409a0bf8..3bf8c7772 100644 --- a/dix/grabs.c +++ b/dix/grabs.c @@ -55,6 +55,7 @@ SOFTWARE. #include "dix/dix_priv.h" #include "dix/dixgrabs_priv.h" #include "dix/exevents_priv.h" +#include "dix/window_priv.h" #include "os/auth.h" #include "os/client_priv.h" @@ -561,7 +562,7 @@ AddPassiveGrabToList(ClientPtr client, GrabPtr pGrab) } } - if (!pGrab->window->optional && !MakeWindowOptional(pGrab->window)) { + if (!MakeWindowOptional(pGrab->window)) { FreeGrab(pGrab); return BadAlloc; } @@ -660,8 +661,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 e5e579f95..3d863344f 100644 --- a/dix/property.c +++ b/dix/property.c @@ -51,6 +51,7 @@ SOFTWARE. #include "dix/dix_priv.h" #include "dix/property_priv.h" +#include "dix/window_priv.h" #include "windowstr.h" #include "propertyst.h" @@ -272,7 +273,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 69d0e729b..9fe306c44 100644 --- a/dix/window.c +++ b/dix/window.c @@ -1338,7 +1338,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; } @@ -1350,7 +1350,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; } @@ -1451,7 +1451,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; } @@ -3447,7 +3447,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 17c1bd4b7..3d9dbf513 100644 --- a/xfixes/region.c +++ b/xfixes/region.c @@ -23,6 +23,7 @@ #include #include "dix/dix_priv.h" +#include "dix/window_priv.h" #include "render/picturestr_priv.h" #include "xfixesint.h" @@ -642,8 +643,8 @@ ProcXFixesSetWindowShapeRegion(ClientPtr client) pRegion = XFixesRegionCopy(pRegion); if (!pRegion) return BadAlloc; - if (!pWin->optional) - MakeWindowOptional(pWin); + if (!MakeWindowOptional(pWin)) + return BadAlloc; switch (stuff->destKind) { default: case ShapeBounding: