Index | Thread | Search

From:
"Ted Unangst" <tedu@tedunangst.com>
Subject:
Re: responsive watch wip
To:
"Job Snijders" <job@sobornost.net>
Cc:
tech@openbsd.org
Date:
Tue, 10 Jun 2025 15:44:34 -0400

Download raw body.

Thread
On 2025-06-10, Job Snijders wrote:

> The correct behaviour is that watch displays the date(1) output every
> 5 seconds. The interval is intended to be "the pause between runs",
> not "execute command every X seconds, duplicates are fine". I hope
> this clarifies.

okay, I think this is closer to that. And removes old read_result.

It still starts a new process when you press space. Maybe it shouldn't,
but maybe it should?

Index: watch.c
===================================================================
RCS file: /home/cvs/src/usr.bin/watch/watch.c,v
diff -u -p -r1.28 watch.c
--- watch.c	31 May 2025 08:26:26 -0000	1.28
+++ watch.c	10 Jun 2025 19:38:34 -0000
@@ -83,6 +83,15 @@ static char	 *cmdstr;
 static size_t	  cmdlen;
 static char	**cmdv;
 
+struct child {
+	char buf[65000];
+	size_t bufsiz;
+	size_t pos;
+	pid_t pid;
+	int fd;
+	struct event evin;
+};
+
 typedef wchar_t	  BUFFER[MAXLINE][MAXCOLUMN + 1];
 BUFFER		  buf0, buf1;
 BUFFER		 *cur_buf, *prev_buf;
@@ -93,17 +102,22 @@ struct event	  ev_timer;
 
 #define ctrl(c)		((c) & 037)
 int display(BUFFER *, BUFFER *, highlight_mode_t);
-void read_result(BUFFER *);
 kbd_result_t kbd_command(int);
 void show_help(void);
 void untabify(wchar_t *, int);
 void on_signal(int, short, void *);
+void on_sigchild(int, short, void *);
 void timer(int, short, void *);
 void input(int, short, void *);
 void quit(void);
 void usage(void);
 void swap_buffers(void);
 
+struct child *running_child;
+void start_child();
+void child_input(int, short, void *);
+void child_done(struct child *);
+
 int
 set_interval(const char *str)
 {
@@ -124,6 +138,7 @@ int
 main(int argc, char *argv[])
 {
 	struct event ev_sigint, ev_sighup, ev_sigterm, ev_sigwinch, ev_stdin;
+	struct event ev_sigchild;
 	size_t len, rem;
 	int i, ch;
 	char *p;
@@ -216,19 +231,19 @@ main(int argc, char *argv[])
 	signal_set(&ev_sighup, SIGHUP, on_signal, NULL);
 	signal_set(&ev_sigterm, SIGTERM, on_signal, NULL);
 	signal_set(&ev_sigwinch, SIGWINCH, on_signal, NULL);
+	signal_set(&ev_sigchild, SIGCHLD, on_sigchild, NULL);
 	signal_add(&ev_sigint, NULL);
 	signal_add(&ev_sighup, NULL);
 	signal_add(&ev_sigterm, NULL);
 	signal_add(&ev_sigwinch, NULL);
+	signal_add(&ev_sigchild, NULL);
 
 	event_set(&ev_stdin, STDIN_FILENO, EV_READ | EV_PERSIST, input, NULL);
 	event_add(&ev_stdin, NULL);
 	evtimer_set(&ev_timer, timer, NULL);
-	evtimer_add(&ev_timer, &opt_interval.tv);
 
 	cur_buf = &buf0; prev_buf = &buf1;
-	read_result(cur_buf);
-	display(cur_buf, prev_buf, highlight_mode);
+	start_child();
 
 	event_dispatch();
 
@@ -379,21 +394,21 @@ display(BUFFER * cur, BUFFER * prev, hig
 }
 
 void
-read_result(BUFFER *buf)
+start_child()
 {
-	FILE	*fp;
-	int	 i, st, fds[2];
-	pid_t	 pipe_pid, pid;
+	struct child *child;
+	int fds[2];
 
-	/* Clear buffer */
-	memset(buf, 0, sizeof(*buf));
+	child = calloc(1, sizeof(*child));
+	child->bufsiz = sizeof(child->buf);
 
 	if (pipe(fds) == -1)
 		err(1, "pipe()");
 
-	if ((pipe_pid = vfork()) == -1)
-		err(1, "vfork()");
-	else if (pipe_pid == 0) {
+	child->pid = vfork();
+	if (child->pid == -1)
+		err(1, "vfork");
+	if (child->pid == 0) {
 		close(fds[0]);
 		if (fds[1] != STDOUT_FILENO) {
 			dup2(fds[1], STDOUT_FILENO);
@@ -407,21 +422,40 @@ read_result(BUFFER *buf)
 		/* use warn(3) + _exit(2) not to call exit(3) */
 		warn("exec(%s)", cmdstr);
 		_exit(1);
-
 		/* NOTREACHED */
 	}
-	if ((fp = fdopen(fds[0], "r")) == NULL)
-		err(1, "fdopen()");
 	close(fds[1]);
+	child->fd = fds[0];
+
+	event_set(&child->evin, child->fd, EV_READ | EV_PERSIST, child_input, child);
+	event_add(&child->evin, NULL);
+
+	running_child = child;
+}
+
+void
+child_done(struct child *child)
+{
+
+	event_del(&child->evin);
+	close(child->fd);
+	free(child);
+	if (running_child == child)
+		running_child = NULL;
+	evtimer_add(&ev_timer, &opt_interval.tv);
+}
+
+void
+on_sigchild(int sig, short event, void *arg)
+{
+	pid_t pid;
+	int st;
 
-	/* Read command output and convert tab to spaces * */
-	for (i = 0; i < MAXLINE && fgetws((*buf)[i], MAXCOLUMN, fp) != NULL;
-	    i++)
-		untabify((*buf)[i], sizeof((*buf)[i]));
-	fclose(fp);
 	do {
-		pid = waitpid(pipe_pid, &st, 0);
+		pid = waitpid(WAIT_ANY, &st, 0);
 	} while (pid == -1 && errno == EINTR);
+	if (running_child && running_child->pid == pid)
+		child_done(running_child);
 
 	/* Remember update time */
 	time(&lastupdate);
@@ -432,6 +466,38 @@ read_result(BUFFER *buf)
 		paused = 1;
 }
 
+void
+child_input(int sig, short event, void *arg)
+{
+	struct child *child = arg;
+	ssize_t n;
+
+	n = read(child->fd, child->buf + child->pos, child->bufsiz - child->pos);
+	if (n == -1) {
+		child_done(child);
+		return;
+	}
+	child->pos += n;
+
+	size_t l = 0, c = 0;
+	BUFFER *buf = cur_buf;
+	memset(*buf, 0, sizeof(*buf));
+	for (size_t i = 0; i < child->pos; i++) {
+		char ch = child->buf[i];
+		if (ch == '\n') {
+			l++;
+			c = 0;
+			if (l == MAXLINE)
+				break;
+			continue;
+		}
+		if (c == MAXCOLUMN)
+			continue;
+		(*buf)[l][c++] = ch;
+	}
+	display(buf, prev_buf, highlight_mode);
+}
+
 kbd_result_t
 kbd_command(int ch)
 {
@@ -534,11 +600,11 @@ kbd_command(int ch)
 	case 'p':
 		if (paused == 1) {
 			paused = 0;
-			evtimer_del(&ev_timer);
+			evtimer_add(&ev_timer, &opt_interval.tv);
 			return (RSLT_REDRAW);
 		} else {
 			paused = 1;
-			evtimer_add(&ev_timer, &opt_interval.tv);
+			evtimer_del(&ev_timer);
 			return (RSLT_UPDATE);
 		}
 
@@ -688,10 +754,8 @@ void
 timer(int sig, short event, void *arg)
 {
 	swap_buffers();
-	read_result(cur_buf);
-	display(cur_buf, prev_buf, highlight_mode);
-	if (!paused)
-		evtimer_add(&ev_timer, &opt_interval.tv);
+	start_child();
+	evtimer_add(&ev_timer, &opt_interval.tv);
 }
 
 void
@@ -705,8 +769,7 @@ input(int sig, short event, void *arg)
 	switch (result) {
 	case RSLT_UPDATE:	/* update buffer */
 		swap_buffers();
-		read_result(cur_buf);
-		display(cur_buf, prev_buf, highlight_mode);
+		start_child();
 		if (!paused)
 			evtimer_add(&ev_timer, &opt_interval.tv);
 		break;