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

[Aaron Plattner]

On 05/17/2012 03:58 PM, Keith Packard wrote:
> 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.

Are you talking about xf86DeleteScreen?  It does that if PreInit fails, 
before it has called AddScreen.

... not that having your screen renumbered between PreInit and 
ScreenInit is something people test a lot...

-- Aaron

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