From ee798cf212388c2a97cbf2f2e60adab370047c71 Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Wed, 2 Oct 2024 22:20:22 +0200 Subject: [PATCH] exa: simplify CreatePixmap()/DestroyPixmap() handlers error pathes Instead of complex wrap/unwrap trickery in the error path, just protect the DestroyPixmap() handlers from half-initialized state. Prior to this change, we always had to make sure, the we're calling the original (screen driver's) handler directly, instead of our own (which calls the original one, too), so it doesn't do any damage. This isn't necessary anymore, since it finds out on it own what to do. This not just makes the code flow simpler and easier to understand, but also clears the road for decoupling the extension specific pixmap destructor logic from the ScreenRec proc vectors (*1). *1) see: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1755 https://gitlab.freedesktop.org/xorg/xserver/-/issues/1754 Signed-off-by: Enrico Weigelt, metux IT consult Part-of: --- exa/exa_classic.c | 21 +++++++++++---------- exa/exa_driver.c | 14 ++++++++------ exa/exa_mixed.c | 7 +++++-- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/exa/exa_classic.c b/exa/exa_classic.c index 7f93f1e3d..05f0ff2a8 100644 --- a/exa/exa_classic.c +++ b/exa/exa_classic.c @@ -96,10 +96,9 @@ exaCreatePixmap_classic(ScreenPtr pScreen, int w, int h, int depth, pExaPixmap->fb_size = pExaPixmap->fb_pitch * h; if (pExaPixmap->fb_pitch > 131071) { - swap(pExaScr, pScreen, DestroyPixmap); - if (pScreen->DestroyPixmap) - pScreen->DestroyPixmap(pPixmap); - swap(pExaScr, pScreen, DestroyPixmap); + // don't need to protect from calling our own (wrapped) DestroyPixmap + // handler, because it can deal with half-initialized state + dixDestroyPixmap(pPixmap, 0); return NULL; } @@ -109,10 +108,9 @@ exaCreatePixmap_classic(ScreenPtr pScreen, int w, int h, int depth, pScreen, pPixmap); if (pExaPixmap->pDamage == NULL) { - swap(pExaScr, pScreen, DestroyPixmap); - if (pScreen->DestroyPixmap) - pScreen->DestroyPixmap(pPixmap); - swap(pExaScr, pScreen, DestroyPixmap); + // don't need to protect from calling our own (wrapped) DestroyPixmap + // handler, because it can deal with half-initialized state + dixDestroyPixmap(pPixmap, 0); return NULL; } @@ -218,6 +216,8 @@ exaDestroyPixmap_classic(PixmapPtr pPixmap) if (pPixmap->refcnt == 1) { ExaPixmapPriv(pPixmap); + if (!pExaPixmap) // we're called on an error path + goto out; exaDestroyPixmap(pPixmap); @@ -235,9 +235,10 @@ exaDestroyPixmap_classic(PixmapPtr pPixmap) RegionUninit(&pExaPixmap->validFB); } +out: + // restore original (screen driver's) DestroyPixmap() handler and call it swap(pExaScr, pScreen, DestroyPixmap); - if (pScreen->DestroyPixmap) - ret = pScreen->DestroyPixmap(pPixmap); + dixDestroyPixmap(pPixmap, 0); swap(pExaScr, pScreen, DestroyPixmap); return ret; diff --git a/exa/exa_driver.c b/exa/exa_driver.c index 17106fcb9..e56f980c7 100644 --- a/exa/exa_driver.c +++ b/exa/exa_driver.c @@ -98,10 +98,9 @@ exaCreatePixmap_driver(ScreenPtr pScreen, int w, int h, int depth, } if (!pExaPixmap->driverPriv) { - swap(pExaScr, pScreen, DestroyPixmap); - if (pScreen->DestroyPixmap) - pScreen->DestroyPixmap(pPixmap); - swap(pExaScr, pScreen, DestroyPixmap); + // don't need to protect from calling our own (wrapped) DestroyPixmap + // handler, because it can deal with half-initialized state + dixDestroyPixmap(pPixmap, 0); return NULL; } @@ -196,6 +195,8 @@ exaDestroyPixmap_driver(PixmapPtr pPixmap) if (pPixmap->refcnt == 1) { ExaPixmapPriv(pPixmap); + if (!pExaPixmap) // we're called on an error path + goto out; exaDestroyPixmap(pPixmap); @@ -204,9 +205,10 @@ exaDestroyPixmap_driver(PixmapPtr pPixmap) pExaPixmap->driverPriv = NULL; } +out: + // restore original (screen driver's) DestroyPixmap() handler and call it swap(pExaScr, pScreen, DestroyPixmap); - if (pScreen->DestroyPixmap) - ret = pScreen->DestroyPixmap(pPixmap); + dixDestroyPixmap(pPixmap, 0); swap(pExaScr, pScreen, DestroyPixmap); return ret; diff --git a/exa/exa_mixed.c b/exa/exa_mixed.c index 7138f9b30..340c45a1f 100644 --- a/exa/exa_mixed.c +++ b/exa/exa_mixed.c @@ -249,6 +249,8 @@ exaDestroyPixmap_mixed(PixmapPtr pPixmap) if (pPixmap->refcnt == 1) { ExaPixmapPriv(pPixmap); + if (!pExaPixmap) + goto out; // we're called on an error path exaDestroyPixmap(pPixmap); @@ -266,9 +268,10 @@ exaDestroyPixmap_mixed(PixmapPtr pPixmap) } } +out: + // restore original (screen driver's) DestroyPixmap() handler and call it swap(pExaScr, pScreen, DestroyPixmap); - if (pScreen->DestroyPixmap) - ret = pScreen->DestroyPixmap(pPixmap); + dixDestroyPixmap(pPixmap, 0); swap(pExaScr, pScreen, DestroyPixmap); return ret;