From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: watch(1): wait for child asynchronously
To: tech <tech@openbsd.org>
Date: Wed, 21 May 2025 13:11:29 +0200

On Wed, May 21, 2025 at 11:46:00AM +0200, Florian Obser wrote:
> OK?

Why? What is the benefit of this?
I don't know watch but you do not explain why we need to do this async.
 
> diff --git watch.c watch.c
> index da4f6c0eacf..3b7c917aa36 100644
> --- watch.c
> +++ watch.c
> @@ -73,6 +73,7 @@ int start_line = 0, start_column = 0;	/* display offset coordinates */
>  int pause_status = 0;		/* pause status */
>  time_t lastupdate;		/* last updated time */
>  int xflag = 0;
> +pid_t pipe_pid;
>  
>  #define	addwch(_x)	addnwstr(&(_x), 1);
>  #define	WCWIDTH(_x)	((wcwidth((_x)) > 0)? wcwidth((_x)) : 1)
> @@ -122,6 +123,7 @@ int
>  main(int argc, char *argv[])
>  {
>  	struct event ev_sigint, ev_sighup, ev_sigterm, ev_sigwinch, ev_stdin;
> +	struct event ev_sigchld;
>  	size_t len, rem;
>  	int i, ch;
>  	char *p;
> @@ -209,10 +211,12 @@ main(int argc, char *argv[])
>  	/*
>  	 * Initialize signal
>  	 */
> +	signal_set(&ev_sigchld, SIGCHLD, on_signal, NULL);
>  	signal_set(&ev_sigint, SIGINT, on_signal, NULL);
>  	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_add(&ev_sigchld, NULL);
>  	signal_add(&ev_sigint, NULL);
>  	signal_add(&ev_sighup, NULL);
>  	signal_add(&ev_sigterm, NULL);
> @@ -374,8 +378,7 @@ void
>  read_result(BUFFER *buf)
>  {
>  	FILE	*fp;
> -	int	 i, st, fds[2];
> -	pid_t	 pipe_pid, pid;
> +	int	 i, fds[2];
>  
>  	/* Clear buffer */
>  	memset(buf, 0, sizeof(*buf));
> @@ -411,12 +414,7 @@ read_result(BUFFER *buf)
>  	    i++)
>  		untabify((*buf)[i], sizeof((*buf)[i]));
>  	fclose(fp);
> -	do {
> -		pid = waitpid(pipe_pid, &st, 0);
> -	} while (pid == -1 && errno == EINTR);
>  
> -	/* Remember update time */
> -	time(&lastupdate);
>  }
>  
>  kbd_result_t
> @@ -644,6 +642,8 @@ void
>  on_signal(int sig, short event, void *arg)
>  {
>  	struct winsize ws;
> +	int status;
> +	pid_t pid;
>  
>  	switch(sig) {
>  	case SIGWINCH:
> @@ -652,6 +652,13 @@ on_signal(int sig, short event, void *arg)
>  		doupdate();
>  		display(cur_buf, prev_buf, highlight_mode);
>  		break;
> +	case SIGCHLD:
> +		pid = waitpid(pipe_pid, &status, WNOHANG);
> +		if (pid == pipe_pid) {
> +			/* Remember update time */
> +			time(&lastupdate);
> +		}
> +		break;
>  	default:
>  		quit();
>  	}
> 
> -- 
> In my defence, I have been left unsupervised.
> 

-- 
:wq Claudio