[PATCH 1/4] xf86: add helper functions to convert to from ScrnInfoPtr/ScreenPtr

Keith Packard keithp at keithp.com
Thu May 17 15:58:10 PDT 2012


Dave Airlie <airlied at gmail.com> writes:

> +
> +ScrnInfoPtr
> +xf86ScreenToScrn(ScreenPtr pScreen)
> +{
> +    return xf86Screens[pScreen->myNum];
> +}

While reviewing, I found a bit of comedy in xf86Helper.c -- it appears
the code 'supports' removing a screen from the middle of the xf86Screens
list. Given that DIX does not support removing screens (at all), this
seems entirely useless.

It does appear that xf86NumScreens is carefully managed enough that you
could stick an assert that pScreen->myNum < xf86NumScreens, although I'm
not sure how useful that would be, given that the DIX screens are
allocated directly from the xf86 scrns.

> +ScreenPtr
> +xf86ScrnToScreen(ScrnInfoPtr pScrn)
> +{
> +    return screenInfo.screens[pScrn->scrnIndex];
> +}

screenInfo.screens is set early in AddScreen, the only function called
after the screen is allocated and before the screen is used is
dixAllocatePrivates.

I'd suggest that you might add an assert that pScrn->scrnIndex <
screenInfo.numScreens -- screenInfo.numScreens is very carefully managed
during init and reset to make this a valuable test, I think. It would
definitely catch code trying to get an X screen before it was allocated.

Reviewed-by: Keith Packard <keithp at keithp.com>

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20120517/0f95ae86/attachment.pgp>


More information about the xorg-devel mailing list