On Mon, 6 Dec 2010 14:53:17 -0800, James Jones <[email protected]> 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.
> - if (IsSystemCounter(pTrigger->pCounter))
> - SyncComputeBracketValues(pTrigger->pCounter);
> + if (SYNC_COUNTER == pTrigger->pSync->type)
> + {
> + pCounter = (SyncCounter *)pTrigger->pSync;
> +
> + if (IsSystemCounter(pCounter))
> + SyncComputeBracketValues(pCounter);
> + }
Seeing changes like this, it's tempting to use a union somewhere so you
could avoid all of these casts.
> @@ -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));
> }
Do we really need asserts in these functions? An error message and the
obvious return might be kinder to the user in case of errors elsewhere
in the server.
> + SyncCounter *pCounter;
> +
> + assert(!pTrigger->pSync || (SYNC_COUNTER == pTrigger->pSync->type));
> +
> + pCounter = (SyncCounter *)pTrigger->pSync;
Btw, I notice you consistently place of the constant on the left side of
the == operator. This isn't common practice in the X server, but I could
grow to like it. Obviously avoids any accidental assignments.
> + assert(!pTrigger->pSync || (SYNC_COUNTER == pTrigger->pSync->type));
Again, we probably don't want an assert here.
(this code looks really nice in general)
Reviewed-by: Keith Packard <[email protected]>
--
[email protected]
pgpvOFXwWqYNE.pgp
Description: PGP signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
