bin/155374: [patch] grdc(6) timing loop still broken

[Andy Farkas]

>Number:         155374
>Category:       bin
>Synopsis:       [patch] grdc(6) timing loop still broken
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Mar 08 12:40:09 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator:     Andy Farkas
>Release:        8.2-STABLE
>Organization:
>Environment:
FreeBSD 8.2-STABLE #0: Mon Mar  7 07:46:31 EST 2011 
>Description:
After adding a small delay loop to the scrolling code of grdc(6) I noticed
that it was not keeping time properly.

This patch adds a delay during scrolling so that you can actually see it,
and also corrects the timing calculations within the do() loop.

The scrolling bit is trivial, see my followup to bin/151663.

The main thrust of this patch is to finally fix the timing of the do() loop:

1/ at the start of the loop we get the current time (now).
2/ we diplay the time, which may take some time, especially if scrolling.
3/ after display we get the time again (delay).
4/ we then calculate the amount of time needed to wait until a whole second has passed (wait).

>How-To-Repeat:
Add a delay during scrolling and notice how the time is displayed wrong.

>Fix:
patch (including scrolling patch):

%%%
--- /usr/src/games/grdc/grdc.c	2010-08-28 13:04:04.000000000 +1000
+++ ./grdc.c	2011-03-08 22:04:00.000000000 +1000
@@ -26,7 +26,12 @@
 #define XLENGTH 58
 #define YDEPTH  7
 
-struct timespec now;
+/* now:		current time we start the loop
+   delay:	current time we finish displaying the time
+   wait:	amount of time we wait until next now second
+   scrold:	amount of time between scrolling
+*/
+struct timespec delay, now, wait, scrold;
 struct tm *tm;
 
 short disp[11] = {
@@ -54,8 +59,6 @@
 int
 main(int argc, char *argv[])
 {
-	struct timespec delay;
-	time_t prev_sec;
 	long t, a;
 	int i, j, s, k;
 	int n;
@@ -69,6 +72,7 @@
 	switch (ch) {
 	case 's':
 		scrol = 1;
+		scrold.tv_nsec = 40000000;
 		break;
 	case 't':
 		t12 = 1;
@@ -138,9 +142,9 @@
 
 		attrset(COLOR_PAIR(2));
 	}
-	clock_gettime(CLOCK_REALTIME_FAST, &now);
-	prev_sec = now.tv_sec;
+
 	do {
+		clock_gettime(CLOCK_REALTIME_FAST, &now);
 		mask = 0;
 		tm = localtime(&now.tv_sec);
 		set(tm->tm_sec%10, 0);
@@ -191,25 +195,31 @@
 				}
 				if(!s) {
 					refresh();
+					nanosleep(&scrold, NULL);
 				}
 			}
 		}
 		movto(6, 0);
 		refresh();
-		clock_gettime(CLOCK_REALTIME_FAST, &now);
-		if (now.tv_sec == prev_sec) {
-			if (delay.tv_nsec > 0) {
-				delay.tv_sec = 0;
-				delay.tv_nsec = 1000000000 - now.tv_nsec;
-			} else {
-				delay.tv_sec = 1;
-				delay.tv_nsec = 0;
+
+		clock_gettime(CLOCK_REALTIME_FAST, &delay);
+
+		if (delay.tv_sec == now.tv_sec) {
+			wait.tv_nsec = 1000000000 - (delay.tv_nsec - now.tv_nsec);
+			--n;
+		}
+		else {
+			if (delay.tv_nsec < now.tv_nsec) {
+				wait.tv_nsec = now.tv_nsec - delay.tv_nsec;
+				n -= delay.tv_sec - now.tv_sec;
+			}
+			else {
+				wait.tv_nsec = (1000000000 - delay.tv_nsec) + now.tv_nsec;
+				n -= (delay.tv_sec - now.tv_sec) + 1;
 			}
-			nanosleep(&delay, NULL);
-			clock_gettime(CLOCK_REALTIME_FAST, &now);
 		}
-		n -= now.tv_sec - prev_sec;
-		prev_sec = now.tv_sec;
+		nanosleep(&wait, NULL);
+
 		if (sigtermed) {
 			standend();
 			clear();


>Release-Note:
>Audit-Trail:
>Unformatted: