xorg-x11-server/fixed-SYNC-extension-trigger-BlockHandler-test.diff

From 6dbae4f4841ddfb70d6f2a973431f34d2b3cc4c0 Mon Sep 17 00:00:00 2001
From: Egbert Eich <eich@freedesktop.org>
Date: Mon, 23 Mar 2009 12:58:28 +0100
Subject: [PATCH] Fixed SYNC extension trigger BlockHandler test.

* Reworked ComputeBracketValues():
  + Removed 'startOver' flag: we always seem to want to start
    over, there doesn't seem to be a reason why we should keep the old
    bracket values, moreover when this flag was set the semantics of
    setting the bracket values for the selected sync counter were flawed
    anyhow:
    the bracket whose value hadn't changed (if the other one did) was simply
    deleted.
  + Rewrote the bracket value calculation for Postitive/NegativeTransition:
    the calulation didn't cover all possible cases and was flawed anyhow.
* Reworked previous patch to IdleTimeBlockHandler()
  (commit: 1f4fb022)
  + Simplified code.
  + Modified heuristics:
    pIdleTimeValueLess does not exclude pIdleTimeValueGreater: if no wakeup
    is scheduled for the former one check if there is one for the latter.
  + If no immediate wakeup is scheduled at all find schedule a wakeup for
    the next time an idle counter might trigger, do not wait for the latest
    idle counter to trigger.
    This fixes a problem introduced with commit 1f4fb022 where an idle counter
    expires unnoticed.
Index: xorg-server-1.6.3.901/Xext/sync.c
===================================================================
--- xorg-server-1.6.3.901.orig/Xext/sync.c
+++ xorg-server-1.6.3.901/Xext/sync.c
@@ -222,8 +222,7 @@ SyncCreateCounter(
 );
 
 static void SyncComputeBracketValues(
-    SyncCounter * /* pCounter */,
-    Bool /* startOver */
+    SyncCounter * /* pCounter */
 );
 
 static void
@@ -333,7 +332,7 @@ SyncDeleteTriggerFromCounter(pTrigger)
     }
     
     if (IsSystemCounter(pTrigger->pCounter))
-	SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE);
+	SyncComputeBracketValues(pTrigger->pCounter);
 }
 
 
@@ -361,7 +360,7 @@ SyncAddTriggerToCounter(pTrigger)
     pTrigger->pCounter->pTriglist = pCur;
 
     if (IsSystemCounter(pTrigger->pCounter))
-	SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE);
+	SyncComputeBracketValues(pTrigger->pCounter);
 
     return Success;
 }
@@ -531,7 +530,7 @@ SyncInitTrigger(client, pTrigger, counte
     }
     else if (IsSystemCounter(pCounter))
     {
-	SyncComputeBracketValues(pCounter, /*startOver*/ TRUE);
+	SyncComputeBracketValues(pCounter);
     }
     
     return Success;
@@ -829,13 +828,14 @@ SyncChangeCounter(pCounter, newval)
     for (ptl = pCounter->pTriglist; ptl; ptl = pnext)
     {
 	pnext = ptl->next;
-	if ((*ptl->pTrigger->CheckTrigger)(ptl->pTrigger, oldval))
+	if ((*ptl->pTrigger->CheckTrigger)(ptl->pTrigger, oldval)) {
 	    (*ptl->pTrigger->TriggerFired)(ptl->pTrigger);
+	}
     }
 
     if (IsSystemCounter(pCounter))
     {
-	SyncComputeBracketValues(pCounter, /* startOver */ FALSE);
+	SyncComputeBracketValues(pCounter);
     }
 }
 
@@ -1117,9 +1117,8 @@ SyncDestroySystemCounter(pSysCounter)
 }
 
 static void
-SyncComputeBracketValues(pCounter, startOver)
+SyncComputeBracketValues(pCounter)
     SyncCounter *pCounter;
-    Bool startOver;
 {
     SyncTriggerList *pCur;
     SyncTrigger *pTrigger;
@@ -1136,58 +1135,53 @@ SyncComputeBracketValues(pCounter, start
     if (ct == XSyncCounterNeverChanges)
 	return;
 
-    if (startOver)
-    {
-	XSyncMaxValue(&psci->bracket_greater);
-	XSyncMinValue(&psci->bracket_less);
-    }
+    XSyncMaxValue(&psci->bracket_greater);
+    XSyncMinValue(&psci->bracket_less);
+
 
     for (pCur = pCounter->pTriglist; pCur; pCur = pCur->next)
     {
 	pTrigger = pCur->pTrigger;
-	
-        if (pTrigger->test_type == XSyncPositiveComparison &&
-	    ct != XSyncCounterNeverIncreases)
-	{
-	    if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) &&
-		XSyncValueLessThan(pTrigger->test_value,
-				   psci->bracket_greater))
-	    {
-		psci->bracket_greater = pTrigger->test_value;
-		pnewgtval = &psci->bracket_greater;
-	    }
-	}
-	else if (pTrigger->test_type == XSyncNegativeComparison &&
-		 ct != XSyncCounterNeverDecreases)
-	{
-	    if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) &&
-		XSyncValueGreaterThan(pTrigger->test_value,
-				      psci->bracket_less))
-	    {
-		psci->bracket_less = pTrigger->test_value;
-		pnewltval = &psci->bracket_less;
-	    }
-	}
-	else if (pTrigger->test_type == XSyncNegativeTransition &&
-		   ct != XSyncCounterNeverIncreases)
-	{
-	    if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) &&
-		XSyncValueGreaterThan(pTrigger->test_value, psci->bracket_less))
-	    {
+
+        if ( ( (pTrigger->test_type == XSyncPositiveComparison)
+	       && ( XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value)
+		    || (ct != XSyncCounterNeverIncreases
+			&& XSyncValueLessThan(pCounter->value, pTrigger->test_value))
+		    )
+	       )
+	     || ((pTrigger->test_type == XSyncPositiveTransition)
+		 && ( ( ct != XSyncCounterNeverDecreases
+			&& XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value))
+		      || ( ct != XSyncCounterNeverIncreases
+			  && XSyncValueLessThan(pCounter->value, pTrigger->test_value))
+		      )
+		 )
+	     ) {
+	      if (XSyncValueLessThan(pTrigger->test_value,
+				     psci->bracket_greater)) {
+		  psci->bracket_greater = pTrigger->test_value;
+		  pnewgtval = &psci->bracket_greater;
+		}
+	} else if ( ( (pTrigger->test_type == XSyncNegativeComparison)
+		      && ( XSyncValueLessOrEqual(pCounter->value, pTrigger->test_value)
+			   || (ct != XSyncCounterNeverDecreases
+			       && XSyncValueGreaterThan(pCounter->value, pTrigger->test_value))
+			   )
+		      )
+		    || ((pTrigger->test_type == XSyncNegativeTransition)
+			&& ( ( ct != XSyncCounterNeverDecreases
+			       && XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value))
+			     || ( ct != XSyncCounterNeverIncreases
+				  && XSyncValueLessThan(pCounter->value, pTrigger->test_value))
+			    )
+			)
+		    ) {
+	    if (XSyncValueGreaterThan(pTrigger->test_value,
+				      psci->bracket_less)) {
 		psci->bracket_less = pTrigger->test_value;
 		pnewltval = &psci->bracket_less;
 	    }
 	}
-        else if (pTrigger->test_type == XSyncPositiveTransition &&
-		  ct != XSyncCounterNeverDecreases)
-	{
-	    if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) &&
-		XSyncValueLessThan(pTrigger->test_value, psci->bracket_greater))
-	    {
-		psci->bracket_greater = pTrigger->test_value;
-		pnewgtval = &psci->bracket_greater;
-	    }
-	}
     } /* end for each trigger */
 
     if (pnewgtval || pnewltval)
@@ -2541,11 +2535,17 @@ IdleTimeQueryValue (pointer pCounter, CA
 }
 
 static void
-IdleTimeBlockHandler(pointer env, struct timeval **wt, pointer LastSelectMask)
-{
-    XSyncValue idle, old_idle;
-    SyncTriggerList *list = IdleTimeCounter->pTriglist;
-    SyncTrigger *trig;
+IdleTimeBlockHandler (pointer env,
+                      struct timeval **wt,
+                      pointer LastSelectMask)
+{
+  XSyncValue idle, old_idle;
+  SyncTriggerList *list = IdleTimeCounter->pTriglist;
+  SyncTrigger *trig;
+  unsigned long timeout = -1;
+  XSyncValue value;
+  Bool overflow;
+ 
 
     if (!pIdleTimeValueLess && !pIdleTimeValueGreater)
 	return;
@@ -2563,46 +2563,57 @@ IdleTimeBlockHandler(pointer env, struct
 	 * want level or edge trigger.  Check the trigger list against the
 	 * current idle time, and if any succeed, bomb out of select()
 	 * immediately so we can reschedule.
+	 * NOTE: we need to check trigger because the idle timer can go
+	 * both ways (XSyncCounterUnrestricted) so that we need to set
+	 * pIdleTimeValueLess in ComputeBracketValues() in the edge
+	 * triggered case even if the idle timer is already less.
 	 */
-
-	for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
+        for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
 	    trig = list->pTrigger;
 	    if (trig->CheckTrigger(trig, old_idle)) {
-		AdjustWaitForDelay(wt, 0);
-		break;
+	        AdjustWaitForDelay(wt, 0);
+		IdleTimeCounter->value = old_idle; /* pop */
+		return;
 	    }
 	}
     }
-    else if (pIdleTimeValueGreater)
+    if (pIdleTimeValueGreater)
     {
-	/*
-	 * There's a threshold in the positive direction.  If we've been
-	 * idle less than it, schedule a wakeup for sometime in the future.
-	 * If we've been idle more than it, and someone wants to know about
-	 * that level-triggered, schedule an immediate wakeup.
-	 */
-	unsigned long timeout = -1;
-
-	if (XSyncValueLessThan (idle, *pIdleTimeValueGreater)) {
-	    XSyncValue value;
-	    Bool overflow;
+      /*
+       * There's a threshold in the positive direction.
+       * If we've been idle more than it, and someone wants to know about
+       * that level-triggered, schedule an immediate wakeup.
+       * NOTE: we need to check trigger because the idle timer can go
+       * both ways (XSyncCounterUnrestricted) so that we need to set
+       * pIdleTimeValueGreater in ComputeBracketValues() in the edge
+       * triggered case even if the idle timer is already greater.
+       */
 
-	    XSyncValueSubtract (&value, *pIdleTimeValueGreater,
-	                        idle, &overflow);
-	    timeout = min(timeout, XSyncValueLow32 (value));
-	} else {
+	if (XSyncValueGreaterOrEqual (idle, *pIdleTimeValueGreater)) {
 	    for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
-		trig = list->pTrigger;
+	        trig = list->pTrigger;
 		if (trig->CheckTrigger(trig, old_idle)) {
-		    timeout = min(timeout, 0);
-		    break;
+		    AdjustWaitForDelay (wt, 0);
+		    IdleTimeCounter->value = old_idle; /* pop */
+		    return;
 		}
 	    }
 	}
-
-	AdjustWaitForDelay (wt, timeout);
     }
 
+    /*
+     * If we don't have to wake up immediately we schedule a wakeup for the
+     * next time a trigger expires.
+     */
+    for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
+        trig = list->pTrigger;
+	if (XSyncValueLessThan (idle, trig->test_value)) {
+	    XSyncValueSubtract (&value, trig->test_value,
+				idle, &overflow);
+	    timeout = min(timeout,XSyncValueLow32 (value));
+	}
+    }
+    AdjustWaitForDelay (wt, timeout);
     IdleTimeCounter->value = old_idle; /* pop */
 }