[PATCHv2] Remove static MAXSCREENS limit from Xext/shm.c.

Jamey Sharp jamey at minilop.net
Fri Oct 2 14:10:22 PDT 2009


Thanks for review, Eric!

On Thu, Oct 01, 2009 at 01:53:15PM -0700, Eric Anholt wrote:
> On Thu, 2009-10-01 at 10:47 -0700, Jamey Sharp wrote:
> > +    drawables = xcalloc(screenInfo.numScreens, sizeof(DrawablePtr));
> 
> Seems like this drawable pointer should be part of the screen private.

That would be nice, but ProcPanoramiXShmGetImage seems to need an actual
array. I didn't dig deeper to determine whether that array could be
eliminated entirely, but I think a change like that would be more
invasive than I'm quite prepared for.

> >  void
> >  ShmRegisterFuncs(ScreenPtr pScreen, ShmFuncsPtr funcs)
> >  {
> > -    shmFuncs[pScreen->myNum] = funcs;
> > +    ShmInitScreenPriv(pScreen)->shmFuncs = funcs;
> >  }
> 
> I think this one might be a GetScreenPriv instead?  I'm guessing that
> ShmExtensionInit happens (set up protocol stuff), then later on DDXes
> call ShmRegisterFuncs on their screen.

That was my first guess too, but it seems to be the other way around.
This was the crash at server startup in my first version of this patch.

> > +	ShmScrPrivateRec *priv;
> >  	pScreen = screenInfo.screens[j];
> >  
> > -	pMap = (*shmFuncs[j]->CreatePixmap)(pScreen, 
> > +	priv = ShmGetScreenPriv(pScreen);
> 
> Stylistically, I like private fetching to be part of the declaration.
> Less chance for early dereferencing.  Also, as many layers have screen
> privates, pixmap privates, gc privates, etc., it can be nice to name the
> variables for the privates screen_priv, pixmap_priv, etc.

I'll fix these and the lack of a CloseScreen cleanup hook and send
another patch. Thanks!

Jamey
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://lists.x.org/archives/xorg-devel/attachments/20091002/52ebb601/attachment.pgp 


More information about the xorg-devel mailing list