From: Siraaj Khandkar <siraaj@khandkar.net>
Date: Tue, 24 Mar 2020 22:54:12 +0000 (-0400)
Subject: Handle retry entirely within the fifo_read_all routine
X-Git-Url: https://git.xandkar.net/?p=khatus.git;a=commitdiff_plain;h=a415999c271885d6baf38c76b6fcf08ec0c0d2b6

Handle retry entirely within the fifo_read_all routine
---

diff --git a/x5/khatus.c b/x5/khatus.c
index 98eea2d..62b3774 100644
--- a/x5/khatus.c
+++ b/x5/khatus.c
@@ -368,13 +368,12 @@ fifo_read_one(Fifo *f, char *buf)
 	/* TODO Record timestamp read */
 }
 
-int
+void
 fifo_read_all(Config *cfg, char *buf)
 {
 	fd_set fds;
 	int maxfd = -1;
-	int reading = 0;  /* Number of FDs with unfinished reads */
-	int ready;
+	int ready = 0;
 	struct stat st;
 
 	FD_ZERO(&fds);
@@ -412,41 +411,43 @@ fifo_read_all(Config *cfg, char *buf)
 	debug("ready: %d\n", ready);
 	assert(ready != 0);
 	if (ready < 0)
+		/* TODO: Do we really want to fail here? */
 		fatal("%s", strerror(errno));
-	for (Fifo *f = cfg->fifos; f; f = f->next) {
-		if (FD_ISSET(f->fd, &fds)) {
-			reading++;
-			debug("reading: %s\n", f->name);
-			switch (fifo_read_one(f, buf)) {
-			/*
-			 * ### MESSAGE LOSS ###
-			 * is introduced by closing at EOM in addition to EOF,
-			 * since there may be unread messages remaining in the
-			 * pipe. However,
-			 *
-			 * ### INTER-MESSAGE PUSHBACK ###
-			 * is also gained, since pipes block at the "open" call.
-			 *
-			 * This is an acceptable trade-off because we are a
-			 * stateless reporter of a _most-recent_ status, not a
-			 * stateful accumulator.
-			 */
-			case END_OF_MESSAGE:
-			case END_OF_FILE:
-			case FAILURE:
-				close(f->fd);
-				f->fd = -1;
-				reading--;
-				break;
-			case RETRY:
-				break;
-			default:
-				assert(0);
+	while (ready) {
+		for (Fifo *f = cfg->fifos; f; f = f->next) {
+			if (FD_ISSET(f->fd, &fds)) {
+				debug("reading: %s\n", f->name);
+				switch (fifo_read_one(f, buf)) {
+				/*
+				 * ### MESSAGE LOSS ###
+				 * is introduced by closing at EOM in addition
+				 * to EOF, since there may be unread messages
+				 * remaining in the pipe. However,
+				 *
+				 * ### INTER-MESSAGE PUSHBACK ###
+				 * is also gained, since pipes block at the
+				 * "open" call.
+				 *
+				 * This is an acceptable trade-off because we
+				 * are a stateless reporter of a _most-recent_
+				 * status, not a stateful accumulator.
+				 */
+				case END_OF_MESSAGE:
+				case END_OF_FILE:
+				case FAILURE:
+					close(f->fd);
+					f->fd = -1;
+					ready--;
+					break;
+				case RETRY:
+					break;
+				default:
+					assert(0);
+				}
 			}
 		}
 	}
-	assert(reading >= 0);
-	return reading;
+	assert(ready == 0);
 }
 
 int
@@ -457,7 +458,6 @@ main(int argc, char *argv[])
 	int seplen = 0;
 	int prefix = 0;
 	int errors = 0;
-	int reading = 0;
 	char *buf;
 	Config cfg0 = defaults;
 	Config *cfg = &cfg0;
@@ -533,8 +533,7 @@ main(int argc, char *argv[])
 		 *       fifo_read_all and desired time of next TTL check.
 		 */
 		/* TODO: How long to wait on IO? Max TTL? */
-		reading = fifo_read_all(cfg, buf);
-		debug("reading: %d\n", reading);
+		fifo_read_all(cfg, buf);
 		if (cfg->output_to_x_root_window) {
 			if (XStoreName(display, DefaultRootWindow(display), buf) < 0)
 				fatal("XStoreName failed.\n");
@@ -546,15 +545,10 @@ main(int argc, char *argv[])
 		clock_gettime(CLOCK_MONOTONIC, &t1); // FIXME: check errors
 		timespecsub(&t1, &t0, &td);
 		debug("td {tv_sec = %ld, tv_nsec = %ld}\n", td.tv_sec, td.tv_nsec);
-		/*
-		 * Skip sleep when we aren't done reading,
-		 * to avoid filling-up the pipe during sleep.
-		 */
-		if (!reading && timespeccmp(&td, &ti, <)) {
+		if (timespeccmp(&td, &ti, <)) {
 			/* Pushback on data producers by refusing to read the
 			 * pipe more frequently than the interval.
 			 */
-			/* FIXME: Client is never blocked after initial open. */
 			timespecsub(&ti, &td, &tc);
 			debug("snooze YES\n");
 			snooze(&tc);