[PATCH xserver (v4) 03/10] Make Await SyncTrigger functions generic

[Peter Hutterer]

On Tue, Dec 07, 2010 at 09:14:29PM -0800, James Jones wrote:
> On Tuesday 07 December 2010 19:53:10 Peter Hutterer wrote:
> > > -SyncInitTrigger(ClientPtr client, SyncTrigger *pTrigger, XSyncCounter
> > > counter, -		Mask changes)
> > > +SyncInitTrigger(ClientPtr client, SyncTrigger *pTrigger, XID syncObject,
> > > +		RESTYPE resType, Mask changes)
> > > 
> > >  {
> > > 
> > > -    SyncCounter *pCounter = pTrigger->pCounter;
> > > +    SyncObject *pSync = pTrigger->pSync;
> > > +    SyncCounter *pCounter = NULL;
> > > 
> > >      int		rc;
> > > 
> > > -    Bool	newcounter = FALSE;
> > > +    Bool	newSyncObject = FALSE;
> > > 
> > >      if (changes & XSyncCACounter)
> > >      {
> > > 
> > > -	if (counter == None)
> > > -	    pCounter = NULL;
> > > -	else if (Success != (rc = dixLookupResourceByType ((pointer
> > > *)&pCounter, -				counter, RTCounter, client, 
> DixReadAccess)))
> > > +	if (syncObject == None)
> > > +	    pSync = NULL;
> > > +	else if (Success != (rc = dixLookupResourceByType ((pointer 
> *)&pSync,
> > > +				syncObject, resType, client, DixReadAccess)))
> > > 
> > >  	{
> > > 
> > > -	    client->errorValue = counter;
> > > +	    client->errorValue = syncObject;
> > > 
> > >  	    return rc;
> > >  	
> > >  	}
> > > 
> > > -	if (pCounter != pTrigger->pCounter)
> > > +	if (pSync != pTrigger->pSync)
> > > 
> > >  	{ /* new counter for trigger */
> > > 
> > > -	    SyncDeleteTriggerFromCounter(pTrigger);
> > > -	    pTrigger->pCounter = pCounter;
> > > -	    newcounter = TRUE;
> > > +	    SyncDeleteTriggerFromSyncObject(pTrigger);
> > > +	    pTrigger->pSync = pSync;
> > > +	    newSyncObject = TRUE;
> > > 
> > >  	}
> > >  	
> > >      }
> > >      
> > >      /* if system counter, ask it what the current value is */
> > > 
> > > -    if (IsSystemCounter(pCounter))
> > > +    if (SYNC_COUNTER == pSync->type)
> > 
> > I get a NULL-pointer dereference here. There's a path where pSync can be
> > NULL:
> > if (changes & XSyncCACounter)
> >         if (syncObject == None)
> >              pSync = NULL;
> > ..
> > if (SYNC_COUNTER == pSync->type) /* boom */
> > 
> > This is triggered on startup each time, I guess by gdm trying to do
> > something with the sync triggers since I can see the gdm cursor for about a
> > second or two before the crash.
> > 
> > Either way, this code path looks like it's missing something.
> 
> Right, Keith ran into the same and committed a fix right away I believe.  Very 
> sorry about the breakage, but the latest bits should work.
> 

confirmed as fixed, thanks. 

Cheers,
  Peter