os: Clean up WaitFor.c

Do all timer stuff before blocking, avoiding a bunch of duplicate code
and merge common code in WaitForSomething.

The WaitForSomething changes need a bit of explanation to show that
the new code is effectively equivalent to the old. Eliding error
checking and trivial bits we've got:

Before:

	if (ready clients)
		timeout = 0
	else
		compute timeout
	i = poll
	if (i <= 0) {
		if (ready clients)
			return TRUE;
		if (input)
			return FALSE;
		if (any ready timers) {
			run timers
			return FALSE;
		}
	} else {
		if (input)
			return FALSE;
		if (any ready timers) {
			run timers
			return FALSE;
		}
		if (ready clients)
			return TRUE;
	}

After:

	if (ready clients)
		timeout = 0;
	else
		compute timeout
		run_timers
	poll

	if (input)
		return FALSE;

	if (ready clients)
		return TRUE;

The old code would return TRUE if there were ready clients and input
pending. Dispatch would then schedule that ready client, but before
processing any requests, it would notice that there was input pending
and go process it. The new code just checks for input first, which is
effectively the same.

If the poll timed out and there weren't clients ready, then timers
would get run.

If the poll didn't time out, then timers would get run, even if there
were clients now ready. Now, if the timeout interval was zero, that
means that the timers must have been ready *before* poll was
invoked. In this case, we should simply run the timers before calling
poll -- no sense calling poll just to discard any data that it
generates.

If the timeout interval was non-zero, and poll didn't timeout, then
either there aren't any timers to run, or we got a surprise and hit a
timer exactly as a client became ready to run. This is the one case
where the new code is different from the old; the new code delays the
timer call until the next time WaitForSomething is called.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>

1 files changed, 41 insertions, 84 deletions

diff --git a/os/WaitFor.c b/os/WaitFor.c
index 334c3fea0..024df3521 100644
--- a/os/WaitFor.c
+++ b/os/WaitFor.c
@@ -121,6 +121,7 @@ struct _OsTimerRec {
 };
 
 static void DoTimer(OsTimerPtr timer, CARD32 now);
+static void DoTimers(CARD32 now);
 static void CheckAllTimers(void);
 static volatile struct xorg_list timers;
 
@@ -133,6 +134,33 @@ first_timer(void)
     return xorg_list_first_entry(&timers, struct _OsTimerRec, list);
 }
 
+/*
+ * Compute timeout until next timer, running
+ * any expired timers
+ */
+static int
+check_timers(void)
+{
+    OsTimerPtr timer;
+
+    while ((timer = first_timer()) != NULL) {
+        CARD32 now = GetTimeInMillis();
+        int timeout = timer->expires - now;
+
+        if (timeout <= 0) {
+            DoTimers(now);
+        } else {
+            /* Make sure the timeout is sane */
+            if (timeout < timer->delta + 250)
+                return timeout;
+
+            /* time has rewound.  reset the timers. */
+            CheckAllTimers();
+        }
+    }
+    return -1;
+}
+
 /*****************
  * WaitForSomething:
  *     Make the server suspend until there is
@@ -158,8 +186,6 @@ WaitForSomething(Bool are_ready)
     int pollerr;
     static Bool were_ready;
     Bool timer_is_running;
-    CARD32 now = 0;
-    OsTimerPtr timer;
 
     timer_is_running = were_ready;
 
@@ -181,27 +207,10 @@ WaitForSomething(Bool are_ready)
         if (workQueue)
             ProcessWorkQueue();
 
-        if (are_ready) {
+        if (are_ready)
             timeout = 0;
-        }
-        else {
-            timeout = -1;
-            if ((timer = first_timer()) != NULL) {
-                now = GetTimeInMillis();
-                timeout = timer->expires - now;
-                if (timeout > 0 && timeout > timer->delta + 250) {
-                    /* time has rewound.  reset the timers. */
-                    CheckAllTimers();
-                    timer = first_timer();
-                }
-
-                if (timer) {
-                    timeout = timer->expires - now;
-                    if (timeout < 0)
-                        timeout = 0;
-                }
-            }
-        }
+        else
+            timeout = check_timers();
 
         BlockHandler(&timeout);
         if (NewOutputPending)
@@ -217,76 +226,24 @@ WaitForSomething(Bool are_ready)
             if (dispatchException)
                 return FALSE;
             if (i < 0) {
-                if (pollerr == EINVAL) {
-                    FatalError("WaitForSomething(): poll: %s\n",
-                               strerror(pollerr));
-                }
-                else if (pollerr != EINTR && pollerr != EAGAIN) {
+                if (pollerr != EINTR && !ETEST(pollerr)) {
                     ErrorF("WaitForSomething(): poll: %s\n",
                            strerror(pollerr));
                 }
             }
-            else if (are_ready) {
-                /*
-                 * If no-one else is home, bail quickly
-                 */
-                break;
-            }
-            if (*checkForInput[0] != *checkForInput[1])
-                return FALSE;
-
-            if ((timer = first_timer()) != NULL) {
-                int expired = 0;
-
-                now = GetTimeInMillis();
-                if ((int) (timer->expires - now) <= 0)
-                    expired = 1;
-
-                if (expired) {
-                    OsBlockSignals();
-                    while ((timer = first_timer()) != NULL && (int) (timer->expires - now) <= 0)
-                        DoTimer(timer, now);
-                    OsReleaseSignals();
-
-                    return FALSE;
-                }
-            }
-        }
-        else {
-            /* check here for DDXes that queue events during Block/Wakeup */
-            if (*checkForInput[0] != *checkForInput[1])
-                return FALSE;
-
-            if ((timer = first_timer()) != NULL) {
-                int expired = 0;
-
-                now = GetTimeInMillis();
-                if ((int) (timer->expires - now) <= 0)
-                    expired = 1;
-
-                if (expired) {
-                    OsBlockSignals();
-                    while ((timer = first_timer()) != NULL && (int) (timer->expires - now) <= 0)
-                        DoTimer(timer, now);
-                    OsReleaseSignals();
+        } else
+            are_ready = clients_are_ready();
 
-                    return FALSE;
-                }
-            }
+        if (*checkForInput[0] != *checkForInput[1])
+            return FALSE;
 
-            are_ready = clients_are_ready();
-            if (are_ready)
-                break;
+        if (are_ready) {
+            were_ready = TRUE;
+            if (!timer_is_running)
+                SmartScheduleStartTimer();
+            return TRUE;
         }
     }
-
-    if (are_ready) {
-        were_ready = TRUE;
-        if (!timer_is_running)
-            SmartScheduleStartTimer();
-    }
-
-    return TRUE;
 }
 
 void