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@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1709>
This commit is contained in:
Enrico Weigelt, metux IT consult 2024-10-02 17:23:10 +02:00 committed by Marge Bot
parent af2d3e9487
commit ef62929f58
7 changed files with 26 additions and 16 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;
}