libast: Improve and harden tvsleep (re: 72968eae) (#186)

On SVR4 platforms, select is a sometimes erroneous wrapper around
the poll system call, so call poll directly for efficiency purposes if
it implies no loss in precision.

See, e.g., http://bugs.motifzone.net/long_list.cgi?buglist=129 .

src/lib/libast/features/tvlib:
- For systems lacking nanosleep, test whether select is truly more
  precise than poll.

src/lib/libast/tm/tvsleep.c:
- Verify that tv argument is not null.
- Immediately return if asked to sleep for a duration of zero.
- Immediately initialize timespec in the nanosleep case.
- Revise variable names to use Apps Hungarian.
- Use poll instead of select when there is no loss in precision.
- Check for overflow in the poll case.
- Improve comments.
- Revise arithmetic to work with unsigned types, rather than
  casting to long.
- Do not return non-zero if we have slept for a sufficient
  time.

src/lib/libast/features/tvlib

@@ -43,6 +43,38 @@ lib	utimets link{
 	}
 }end
 
+tst	prefer_poll note{ is select less precise than 1 ms }end execute{
+	#include <sys/types.h>
+	#include <sys/time.h>
+	#if _sys_select
+	# include <sys/select.h>
+	#else
+	# include <sys/socket.h>
+	#endif
+	int
+	main()
+	{
+		struct timeval tvSleep = { 0, 1 }, tvBefore, tvAfter;
+		int i;
+		/* Take a few measurements to prevent a fluke. */
+		for (i = 0; i < 5; ++i)
+		{
+			gettimeofday(&tvBefore, NULL);
+			select(0, NULL, NULL, NULL, &tvSleep);
+			gettimeofday(&tvAfter, NULL);
+			if (tvAfter.tv_sec == tvBefore.tv_sec + 1)
+			{
+				--tvAfter.tv_sec;
+				tvAfter.tv_usec += 1000000;
+			}
+			if ((tvAfter.tv_sec == tvBefore.tv_sec) &&
+				tvAfter.tv_usec - tvBefore.tv_usec < 1000)
+				return -1;
+		}
+		return 0;
+	}
+}end
+
 tst	- -DN=1 - -DN=2 - -DN=3 - -DN=4 output{
 	#include <sys/types.h>
 	#include <sys/time.h>

src/lib/libast/tm/tvsleep.c

@@ -21,14 +21,15 @@
 ***********************************************************************/
 #pragma prototyped
 
+#include <assert.h>
 #include <tv.h>
 #include <tm.h>
 #include <error.h>
 
 #include "FEATURE/tvlib"
 
-#if !_lib_nanosleep && !_lib_usleep
+#if !_lib_nanosleep
-# if _lib_select
+# if _lib_select && !_prefer_poll
 #  if _sys_select
 #   include <sys/select.h>
 #  else
@@ -41,7 +42,8 @@
 # endif
 #endif
 
-#define NANOSECONDS 1000000000L
+#define NANOSECONDS  1000000000L
+#define MILLISECONDS 1000
 
 /*
  * sleep for tv
@@ -54,16 +56,21 @@
 int
 tvsleep(register const Tv_t* tv, register Tv_t* rv)
 {
+	assert(tv); /* Validate argument */
 
-	int		r;
+	/* Return immediately if asked to sleep for no duration. */
+	if (!tv->tv_sec && !tv->tv_nsec)
+		return 0;
+
+	{
 
 #if _lib_nanosleep
 
-	struct timespec	stv;
+	/* PRECISION: nanoseconds */
+	struct timespec	stv = { tv->tv_sec, tv->tv_nsec };
 	struct timespec	srv;
+	int r;
 
-	stv.tv_sec = tv->tv_sec;
-	stv.tv_nsec = tv->tv_nsec;
 	if ((r = nanosleep(&stv, &srv)) && errno == EINTR && rv)
 	{
 		rv->tv_sec = srv.tv_sec;
@@ -71,20 +78,45 @@ tvsleep(register const Tv_t* tv, register Tv_t* rv)
 	}
 	return r;
 
+	}
+
 #else
 
-	Tv_t		bv;
+	Tv_t tvBefore;
 
-	tvgettime(&bv);
+	tvgettime(&tvBefore);
 	{
 
-#if _lib_select
+#if _lib_select && !_prefer_poll
 
+	/* PRECISION: microseconds */
 	struct timeval tvSleep = { tv->tv_sec, tv->tv_nsec / 1000 };
 	if (tv->tv_nsec % 1000)
 		++tvSleep.tv_usec;
+	(void)select(0, NiL, NiL, NiL, &tvSleep);
 
-	r = select(0, NiL, NiL, NiL, &tvSleep);
+#elif _lib_poll
+
+	/* PRECISION: milliseconds
+	 *
+	 * We can sleep for up to 24 days with a single call to poll
+	 * on systems with 32-bit integers, so calling sleep first
+	 * only worsens precision with little gain. For example, the
+	 * UnixWare manual page for usleep warns that a single call of
+	 * that function requires eight system calls, but poll only
+	 * requires one.
+	 */
+	struct pollfd	dummy;
+	int		timeout = INT_MAX;  /* expose bugs */
+
+	if (tv->tv_sec <= (INT_MAX / MILLISECONDS))
+	{
+		timeout = tv->tv_sec  * MILLISECONDS +
+			  tv->tv_nsec / 1000000;
+		if (timeout < INT_MAX && tv->tv_nsec % 1000000)
+			++timeout;
+	}
+	(void)poll(&dummy, 0, timeout);
 
 #else
 
@@ -156,59 +188,71 @@ tvsleep(register const Tv_t* tv, register Tv_t* rv)
 		}
 	}
 
-#elif _lib_poll
-
-	struct pollfd		pfd;
-	int			t;
-
-	if (!(t = (n + 999999L) / 1000000L))
-		t = 1;
-	r = poll(&pfd, 0, t);
-
 #endif
-
 #endif
 
 	}
 
-/* Ascertain whether we actually slept for a sufficient time.
+/* Unfortunately, some operating systems return success for select
- * It is preferable to sleep a little more than necessary than too little.
+ * or poll without having slept for the specified duration, so check
+ * the clock.
+ *
+ * Although time discrepancies when sleeping are inevitable, tvsleep
+ * can guarantee that they always lie on or after the time specified,
+ * which is much more useful from the point of view of predictability
+ * than if they could also occur before.
  */
 	{
-		struct timespec tsAfter;
+		Tv_t tvAfter;
-		long sec, nsec;
 
-		tvgettime(&tsAfter);
+		tvgettime(&tvAfter);
 
-		/* Calculate seconds left to sleep */
+		if (tvAfter.tv_nsec < tvBefore.tv_nsec)
-		sec = (long)(tv->tv_sec + tv->tv_nsec / NANOSECONDS) -
-			((long)tsAfter.tv_sec - (long)bv.tv_sec);
-
-		/* Calculate nanoseconds left to sleep */
-		nsec = (long)(tv->tv_nsec % NANOSECONDS) -
-			((long)tsAfter.tv_nsec - (long)bv.tv_nsec);
-		if (nsec >= NANOSECONDS)
 		{
-			++sec;
+			if (!tvAfter.tv_sec)
-			nsec -= NANOSECONDS;
+				return 0;
+			--tvAfter.tv_sec;
+			tvAfter.tv_nsec += (NANOSECONDS - tvBefore.tv_nsec);
 		}
-		else if (nsec < 0)
+		else
 		{
-			--sec;
+		    tvAfter.tv_nsec -= tvBefore.tv_nsec;
-			nsec += NANOSECONDS;
 		}
+		if (tvAfter.tv_sec < tvBefore.tv_sec)
+			return 0;
+		tvAfter.tv_sec -= tvBefore.tv_sec;
+		/* tvAfter now holds the non-negative time slept */
 
-		if (sec >= 0 && (sec > 0 || nsec > 0))
+		tvBefore = *tv;
+		/* Normalize the time to sleep so that ns < 10e9 */
+		tvBefore.tv_sec  += tvBefore.tv_nsec / NANOSECONDS;
+		tvBefore.tv_nsec %= NANOSECONDS;
+
+		if (tvBefore.tv_nsec < tvAfter.tv_nsec)
+		{
+			if (!tvBefore.tv_sec)
+				return 0;
+			--tvBefore.tv_sec;
+			tvBefore.tv_nsec += (NANOSECONDS - tvAfter.tv_nsec);
+		}
+		else
+		{
+			tvBefore.tv_nsec -= tvAfter.tv_nsec;
+		}
+		if (tvBefore.tv_sec < tvAfter.tv_sec)
+			return 0;
+		tvBefore.tv_sec -= tvAfter.tv_sec;
+
+		if (tvBefore.tv_sec > 0 || tvBefore.tv_nsec > 0)
 		{
 			if (rv)
-			{
+				*rv = tvBefore;
-				rv->tv_sec  = sec;
-				rv->tv_nsec = nsec;
-			}
 			return -1;
 		}
 	}
-	return r;
+	}
+
+	return 0;
 
 #endif