From: "Ted Unangst" Subject: Re: responsive watch wip To: "Job Snijders" Cc: tech@openbsd.org Date: Tue, 10 Jun 2025 15:44:34 -0400 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;