On Mon, Dec 06, 2010 at 02:53:17PM -0800, James Jones wrote:
> Update all the functions dealing with Await
> sync triggers handle generic sync objects
> instead of just counters. This will
> facilitate code sharing between the counter
> sync waits and the fence sync waits.
>
> Signed-off-by: James Jones <[email protected]>
> ---
> Xext/sync.c | 308
> ++++++++++++++++++++++++++++++++++++--------------------
> Xext/syncsrv.h | 2 +-
> 2 files changed, 198 insertions(+), 112 deletions(-)
>
> diff --git a/Xext/sync.c b/Xext/sync.c
> index d5187dd..1a2d55d 100644
> --- a/Xext/sync.c
> +++ b/Xext/sync.c
> @@ -107,18 +107,19 @@ static void SyncInitIdleTime(void);
> * delete and add triggers on this list.
> */
> static void
> -SyncDeleteTriggerFromCounter(SyncTrigger *pTrigger)
> +SyncDeleteTriggerFromSyncObject(SyncTrigger *pTrigger)
> {
> SyncTriggerList *pCur;
> SyncTriggerList *pPrev;
> + SyncCounter *pCounter;
>
> - /* pCounter needs to be stored in pTrigger before calling here. */
> + /* pSync needs to be stored in pTrigger before calling here. */
>
> - if (!pTrigger->pCounter)
> + if (!pTrigger->pSync)
> return;
>
> pPrev = NULL;
> - pCur = pTrigger->pCounter->sync.pTriglist;
> + pCur = pTrigger->pSync->pTriglist;
>
> while (pCur)
> {
> @@ -127,7 +128,7 @@ SyncDeleteTriggerFromCounter(SyncTrigger *pTrigger)
> if (pPrev)
> pPrev->next = pCur->next;
> else
> - pTrigger->pCounter->sync.pTriglist = pCur->next;
> + pTrigger->pSync->pTriglist = pCur->next;
>
> free(pCur);
> break;
> @@ -137,21 +138,27 @@ SyncDeleteTriggerFromCounter(SyncTrigger *pTrigger)
> pCur = pCur->next;
> }
>
> - if (IsSystemCounter(pTrigger->pCounter))
> - SyncComputeBracketValues(pTrigger->pCounter);
> + if (SYNC_COUNTER == pTrigger->pSync->type)
> + {
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + if (IsSystemCounter(pCounter))
> + SyncComputeBracketValues(pCounter);
> + }
> }
>
>
> static int
> -SyncAddTriggerToCounter(SyncTrigger *pTrigger)
> +SyncAddTriggerToSyncObject(SyncTrigger *pTrigger)
> {
> SyncTriggerList *pCur;
> + SyncCounter *pCounter;
>
> - if (!pTrigger->pCounter)
> + if (!pTrigger->pSync)
> return Success;
>
> /* don't do anything if it's already there */
> - for (pCur = pTrigger->pCounter->sync.pTriglist; pCur; pCur = pCur->next)
> + for (pCur = pTrigger->pSync->pTriglist; pCur; pCur = pCur->next)
> {
> if (pCur->pTrigger == pTrigger)
> return Success;
> @@ -161,11 +168,16 @@ SyncAddTriggerToCounter(SyncTrigger *pTrigger)
> return BadAlloc;
>
> pCur->pTrigger = pTrigger;
> - pCur->next = pTrigger->pCounter->sync.pTriglist;
> - pTrigger->pCounter->sync.pTriglist = pCur;
> + pCur->next = pTrigger->pSync->pTriglist;
> + pTrigger->pSync->pTriglist = pCur;
>
> - if (IsSystemCounter(pTrigger->pCounter))
> - SyncComputeBracketValues(pTrigger->pCounter);
> + if (SYNC_COUNTER == pTrigger->pSync->type)
> + {
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + if (IsSystemCounter(pCounter))
> + SyncComputeBracketValues(pCounter);
> + }
>
> return Success;
> }
> @@ -188,69 +200,91 @@ SyncAddTriggerToCounter(SyncTrigger *pTrigger)
> static Bool
> SyncCheckTriggerPositiveComparison(SyncTrigger *pTrigger, CARD64 oldval)
> {
> - return (pTrigger->pCounter == NULL ||
> - XSyncValueGreaterOrEqual(pTrigger->pCounter->value,
> - pTrigger->test_value));
> + SyncCounter *pCounter;
> +
> + assert(!pTrigger->pSync || (SYNC_COUNTER == pTrigger->pSync->type));
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + return (pCounter == NULL ||
> + XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value));
> }
>
> static Bool
> SyncCheckTriggerNegativeComparison(SyncTrigger *pTrigger, CARD64 oldval)
> {
> - return (pTrigger->pCounter == NULL ||
> - XSyncValueLessOrEqual(pTrigger->pCounter->value,
> - pTrigger->test_value));
> + SyncCounter *pCounter;
> +
> + assert(!pTrigger->pSync || (SYNC_COUNTER == pTrigger->pSync->type));
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + return (pCounter == NULL ||
> + XSyncValueLessOrEqual(pCounter->value, pTrigger->test_value));
> }
>
> static Bool
> SyncCheckTriggerPositiveTransition(SyncTrigger *pTrigger, CARD64 oldval)
> {
> - return (pTrigger->pCounter == NULL ||
> + SyncCounter *pCounter;
> +
> + assert(!pTrigger->pSync || (SYNC_COUNTER == pTrigger->pSync->type));
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + return (pCounter == NULL ||
> (XSyncValueLessThan(oldval, pTrigger->test_value) &&
> - XSyncValueGreaterOrEqual(pTrigger->pCounter->value,
> - pTrigger->test_value)));
> + XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value)));
> }
>
> static Bool
> SyncCheckTriggerNegativeTransition(SyncTrigger *pTrigger, CARD64 oldval)
> {
> - return (pTrigger->pCounter == NULL ||
> + SyncCounter *pCounter;
> +
> + assert(!pTrigger->pSync || (SYNC_COUNTER == pTrigger->pSync->type));
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + return (pCounter == NULL ||
> (XSyncValueGreaterThan(oldval, pTrigger->test_value) &&
> - XSyncValueLessOrEqual(pTrigger->pCounter->value,
> - pTrigger->test_value)));
> + XSyncValueLessOrEqual(pCounter->value, pTrigger->test_value)));
> }
>
> static int
> -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.
Cheers,
Peter
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
