xserver: Branch 'master' - 2 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Feb 6 23:08:08 UTC 2025


 Xext/shm.c            |    5 +++--
 Xext/xvmain.c         |    5 +++--
 dix/dispatch.c        |    5 +++--
 exa/exa_classic.c     |   11 +++++++----
 exa/exa_driver.c      |    8 +++++---
 exa/exa_mixed.c       |    5 +++--
 glamor/glamor_egl.c   |    5 +++--
 miext/damage/damage.c |    3 ++-
 8 files changed, 29 insertions(+), 18 deletions(-)

New commits:
commit 24d693c5ae89fa10b1204ffaea128c4a7e2dc25f
Author: Enrico Weigelt, metux IT consult <info at metux.net>
Date:   Mon Sep 30 17:36:05 2024 +0200

    dix: make dixDestroyPixmap() NULL-proof
    
    Make dixDestroyPixmap() check for NULL pointer, so callers don't need to
    do it anymore. Returning TRUE on NULL pointer - but most callers won't
    even look at the retval anyways.
    
    Together with subsequent commits, which will make use of that function,
    instead of calling raw ScreenRec->DestroyPixmap vectors, this gives us some
    more freedom for architectural changes, eg. get rid of the extremely
    complicated and fragile wrapping chains.
    
    Signed-off-by: Enrico Weigelt, metux IT consult <info at metux.net>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1709>

diff --git a/dix/dispatch.c b/dix/dispatch.c
index 1c40bd2ca..5f7cfe02d 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -1462,8 +1462,9 @@ int
 dixDestroyPixmap(void *value, XID pid)
 {
     PixmapPtr pPixmap = (PixmapPtr) value;
-
-    return (*pPixmap->drawable.pScreen->DestroyPixmap) (pPixmap);
+    if (pPixmap && pPixmap->drawable.pScreen && pPixmap->drawable.pScreen->DestroyPixmap)
+        return pPixmap->drawable.pScreen->DestroyPixmap(pPixmap);
+    return TRUE;
 }
 
 int
commit ef62929f581bac5ff77a04c97cb45fb1164bcb4b
Author: Enrico Weigelt, metux IT consult <info at metux.net>
Date:   Wed Oct 2 17:23:10 2024 +0200

    treewide: NULL-protect ScreenRec->DestroyPixmap() calls
    
    Right now, we're assuming that even when deep nesting involved, the proc
    vector is always set to a valid function. One the one hand it requires
    extra dummy procs in some cases, OTOH it's making upcoming refactoring
    of the code flow unnecessarily complex.
    
    The big plot (of subsequent commits) is splitting out the extension's
    (and possibly subsystem's) special logic out of the wrapping chain and
    let them be executed independently from the DDX/drivers - when applicable
    even only when the pixmap is really destroyed (not just unref'ed).
    (At some later point, it might even become be actually a valid situation
    that DestroyPixmap vector really being NULL.)
    
    See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1754
    See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1755
    
    Signed-off-by: Enrico Weigelt, metux IT consult <info at metux.net>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1709>

diff --git a/Xext/shm.c b/Xext/shm.c
index 2fba1820a..beb30040b 100644
--- a/Xext/shm.c
+++ b/Xext/shm.c
@@ -250,13 +250,14 @@ ShmDestroyPixmap(PixmapPtr pPixmap)
     ScreenPtr pScreen = pPixmap->drawable.pScreen;
     ShmScrPrivateRec *screen_priv = ShmGetScreenPriv(pScreen);
     void *shmdesc = NULL;
-    Bool ret;
+    Bool ret = TRUE;
 
     if (pPixmap->refcnt == 1)
         shmdesc = dixLookupPrivate(&pPixmap->devPrivates, shmPixmapPrivateKey);
 
     pScreen->DestroyPixmap = screen_priv->destroyPixmap;
-    ret = (*pScreen->DestroyPixmap) (pPixmap);
+    if (pScreen->DestroyPixmap)
+        ret = pScreen->DestroyPixmap(pPixmap);
     screen_priv->destroyPixmap = pScreen->DestroyPixmap;
     pScreen->DestroyPixmap = ShmDestroyPixmap;
 
diff --git a/Xext/xvmain.c b/Xext/xvmain.c
index 5ff2692f1..bc4a7489d 100644
--- a/Xext/xvmain.c
+++ b/Xext/xvmain.c
@@ -371,13 +371,14 @@ static Bool
 XvDestroyPixmap(PixmapPtr pPix)
 {
     ScreenPtr pScreen = pPix->drawable.pScreen;
-    Bool status;
+    Bool status = TRUE;
 
     if (pPix->refcnt == 1)
         XvStopAdaptors(&pPix->drawable);
 
     SCREEN_PROLOGUE(pScreen, DestroyPixmap);
-    status = (*pScreen->DestroyPixmap) (pPix);
+    if (pScreen->DestroyPixmap)
+        status = pScreen->DestroyPixmap(pPix);
     SCREEN_EPILOGUE(pScreen, DestroyPixmap, XvDestroyPixmap);
 
     return status;
diff --git a/exa/exa_classic.c b/exa/exa_classic.c
index 1b70d350e..7f93f1e3d 100644
--- a/exa/exa_classic.c
+++ b/exa/exa_classic.c
@@ -97,7 +97,8 @@ exaCreatePixmap_classic(ScreenPtr pScreen, int w, int h, int depth,
 
     if (pExaPixmap->fb_pitch > 131071) {
         swap(pExaScr, pScreen, DestroyPixmap);
-        pScreen->DestroyPixmap(pPixmap);
+        if (pScreen->DestroyPixmap)
+            pScreen->DestroyPixmap(pPixmap);
         swap(pExaScr, pScreen, DestroyPixmap);
         return NULL;
     }
@@ -109,7 +110,8 @@ exaCreatePixmap_classic(ScreenPtr pScreen, int w, int h, int depth,
 
     if (pExaPixmap->pDamage == NULL) {
         swap(pExaScr, pScreen, DestroyPixmap);
-        pScreen->DestroyPixmap(pPixmap);
+        if (pScreen->DestroyPixmap)
+            pScreen->DestroyPixmap(pPixmap);
         swap(pExaScr, pScreen, DestroyPixmap);
         return NULL;
     }
@@ -212,7 +214,7 @@ exaDestroyPixmap_classic(PixmapPtr pPixmap)
     ScreenPtr pScreen = pPixmap->drawable.pScreen;
 
     ExaScreenPriv(pScreen);
-    Bool ret;
+    Bool ret = TRUE;
 
     if (pPixmap->refcnt == 1) {
         ExaPixmapPriv(pPixmap);
@@ -234,7 +236,8 @@ exaDestroyPixmap_classic(PixmapPtr pPixmap)
     }
 
     swap(pExaScr, pScreen, DestroyPixmap);
-    ret = pScreen->DestroyPixmap(pPixmap);
+    if (pScreen->DestroyPixmap)
+        ret = pScreen->DestroyPixmap(pPixmap);
     swap(pExaScr, pScreen, DestroyPixmap);
 
     return ret;
diff --git a/exa/exa_driver.c b/exa/exa_driver.c
index 4b7a53f7e..17106fcb9 100644
--- a/exa/exa_driver.c
+++ b/exa/exa_driver.c
@@ -99,7 +99,8 @@ exaCreatePixmap_driver(ScreenPtr pScreen, int w, int h, int depth,
 
     if (!pExaPixmap->driverPriv) {
         swap(pExaScr, pScreen, DestroyPixmap);
-        pScreen->DestroyPixmap(pPixmap);
+        if (pScreen->DestroyPixmap)
+            pScreen->DestroyPixmap(pPixmap);
         swap(pExaScr, pScreen, DestroyPixmap);
         return NULL;
     }
@@ -191,7 +192,7 @@ exaDestroyPixmap_driver(PixmapPtr pPixmap)
     ScreenPtr pScreen = pPixmap->drawable.pScreen;
 
     ExaScreenPriv(pScreen);
-    Bool ret;
+    Bool ret = TRUE;
 
     if (pPixmap->refcnt == 1) {
         ExaPixmapPriv(pPixmap);
@@ -204,7 +205,8 @@ exaDestroyPixmap_driver(PixmapPtr pPixmap)
     }
 
     swap(pExaScr, pScreen, DestroyPixmap);
-    ret = pScreen->DestroyPixmap(pPixmap);
+    if (pScreen->DestroyPixmap)
+        ret = pScreen->DestroyPixmap(pPixmap);
     swap(pExaScr, pScreen, DestroyPixmap);
 
     return ret;
diff --git a/exa/exa_mixed.c b/exa/exa_mixed.c
index f1aeacd48..7138f9b30 100644
--- a/exa/exa_mixed.c
+++ b/exa/exa_mixed.c
@@ -245,7 +245,7 @@ exaDestroyPixmap_mixed(PixmapPtr pPixmap)
     ScreenPtr pScreen = pPixmap->drawable.pScreen;
 
     ExaScreenPriv(pScreen);
-    Bool ret;
+    Bool ret = TRUE;
 
     if (pPixmap->refcnt == 1) {
         ExaPixmapPriv(pPixmap);
@@ -267,7 +267,8 @@ exaDestroyPixmap_mixed(PixmapPtr pPixmap)
     }
 
     swap(pExaScr, pScreen, DestroyPixmap);
-    ret = pScreen->DestroyPixmap(pPixmap);
+    if (pScreen->DestroyPixmap)
+        ret = pScreen->DestroyPixmap(pPixmap);
     swap(pExaScr, pScreen, DestroyPixmap);
 
     return ret;
diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
index bb5e4d658..668eddc86 100644
--- a/glamor/glamor_egl.c
+++ b/glamor/glamor_egl.c
@@ -764,7 +764,7 @@ glamor_egl_destroy_pixmap(PixmapPtr pixmap)
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
     struct glamor_egl_screen_private *glamor_egl =
         glamor_egl_get_screen_private(scrn);
-    Bool ret;
+    Bool ret = TRUE;
 
     if (pixmap->refcnt == 1) {
         struct glamor_pixmap_private *pixmap_priv =
@@ -775,7 +775,8 @@ glamor_egl_destroy_pixmap(PixmapPtr pixmap)
     }
 
     screen->DestroyPixmap = glamor_egl->saved_destroy_pixmap;
-    ret = screen->DestroyPixmap(pixmap);
+    if (screen->DestroyPixmap)
+        ret = screen->DestroyPixmap(pixmap);
     glamor_egl->saved_destroy_pixmap = screen->DestroyPixmap;
     screen->DestroyPixmap = glamor_egl_destroy_pixmap;
 
diff --git a/miext/damage/damage.c b/miext/damage/damage.c
index 592f07bfe..0b626eba4 100644
--- a/miext/damage/damage.c
+++ b/miext/damage/damage.c
@@ -1502,7 +1502,8 @@ damageDestroyPixmap(PixmapPtr pPixmap)
         }
     }
     unwrap(pScrPriv, pScreen, DestroyPixmap);
-    (*pScreen->DestroyPixmap) (pPixmap);
+    if (pScreen->DestroyPixmap)
+        pScreen->DestroyPixmap(pPixmap);
     wrap(pScrPriv, pScreen, DestroyPixmap, damageDestroyPixmap);
     return TRUE;
 }


More information about the xorg-commit mailing list