Re: {relayd,httpd}/proc.c: sync trivial bits

Rafael Sadowski <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
On Thu Jul 02, 2026 at 10:04:35AM +0200, Martijn van Duren wrote:
> Hello tech@,
> 
> This syncs mostly whitespace, comments, and fatal() messages between
> httpd and relayd. The only functional change is a conditional inside
> proc_kill(), which is also present in iked/snmpd.
> 
> With this diff in addition to the control diff send earlier the only
> relevant difference between the two is a conditional preventing sharing
> of socketpairs.
> 
> OK?

Hi Martijn

The most recent formatting changes were made by knfmt(1) in relayd. I
would prefer to keep them, as I think they make sense and I would like a
knfmt(1) workflow that allows me(tm) to simply avoid formatting errors and
achieve clean, consistent formatting.

In my view, therefore, only the __func__ fatal make sense.

On the other hand I know where you're going with this.

> 
> martijn@
> 
> diff /usr/src
> path + /usr/src
> commit - fb32c8576ecb0ad5dc3bca5db208b3b6d6f92346
> blob - 5b952631d4d6f44cda5c614c89f4effe5ae99f31
> file + usr.sbin/httpd/proc.c
> --- usr.sbin/httpd/proc.c
> +++ usr.sbin/httpd/proc.c
> @@ -94,7 +94,7 @@ proc_exec(struct privsep *ps, struct privsep_proc *pro
>  	/* Point process instance arg to stack and copy the original args. */
>  	nargv[nargc++] = "-I";
>  	nargv[nargc++] = num;
> -	for (i = 1; i < (unsigned int) argc; i++)
> +	for (i = 1; i < (unsigned int)argc; i++)
>  		nargv[nargc++] = argv[i];
>  
>  	nargv[nargc] = NULL;
> @@ -158,7 +158,7 @@ proc_connect(struct privsep *ps)
>  			iev = &ps->ps_ievs[dst][inst];
>  			if (imsgbuf_init(&iev->ibuf,
>  			    ps->ps_pp->pp_pipes[dst][inst]) == -1)
> -				fatal(NULL);
> +				fatal("%s: imsgbuf_init", __func__);
>  			imsgbuf_allow_fdpass(&iev->ibuf);
>  			event_set(&iev->ev, iev->ibuf.fd, iev->events,
>  			    iev->handler, iev->data);
> @@ -271,7 +271,7 @@ proc_accept(struct privsep *ps, int fd, enum privsep_p
>  
>  	iev = &ps->ps_ievs[dst][n];
>  	if (imsgbuf_init(&iev->ibuf, fd) == -1)
> -		fatal(NULL);
> +		fatal("%s: imsgbuf_init", __func__);
>  	imsgbuf_allow_fdpass(&iev->ibuf);
>  	event_set(&iev->ev, iev->ibuf.fd, iev->events, iev->handler, iev->data);
>  	event_add(&iev->ev, NULL);
> @@ -385,7 +385,7 @@ proc_kill(struct privsep *ps)
>  			free(cause);
>  		} else
>  			log_warnx("lost child: pid %u", pid);
> -	} while (pid != -1 || errno == EINTR);
> +	} while (pid != -1 || (pid == -1 && errno == EINTR));
>  }
>  
>  void
> @@ -435,7 +435,7 @@ proc_open(struct privsep *ps, int src, int dst)
>  			 */
>  			if (proc_flush_imsg(ps, src, i) == -1 ||
>  			    proc_flush_imsg(ps, dst, j) == -1)
> -				fatal("%s: imsgbuf_flush", __func__);
> +				fatal("%s: proc_flush_imsg", __func__);
>  		}
>  	}
>  }
> @@ -603,13 +603,12 @@ proc_dispatch(int fd, short event, void *arg)
>  
>  	if (event & EV_WRITE) {
>  		if (imsgbuf_write(ibuf) == -1) {
> -			if (errno == EPIPE) {	/* connection closed */
> -				/* remove the event handler */
> +			if (errno == EPIPE) {	/* Connection closed. */
>  				event_del(&iev->ev);
>  				event_loopexit(NULL);
>  				return;
> -			} else
> -				fatal("%s: imsgbuf_write", __func__);
> +			}
> +			fatal("%s: imsgbuf_write", __func__);
>  		}
>  	}
>  
> @@ -641,7 +640,7 @@ proc_dispatch(int fd, short event, void *arg)
>  		switch (imsg_get_type(&imsg)) {
>  		case IMSG_CTL_VERBOSE:
>  			if (imsg_get_data(&imsg, &ver, sizeof(ver)) == -1)
> -			       fatalx("%s: imsg_get_data", __func__);
> +				fatalx("%s: imsg_get_data", __func__);
>  
>  			log_setverbose(ver);
>  			break;
> @@ -656,7 +655,7 @@ proc_dispatch(int fd, short event, void *arg)
>  				fatalx("%s: imsg_get_data", __func__);
>  
>  			proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
> -				    pf.pf_instance);
> +			    pf.pf_instance);
>  			break;
>  		default:
>  			fatalx("%s: %s %d got invalid imsg %d peerid %d "
> commit - fb32c8576ecb0ad5dc3bca5db208b3b6d6f92346
> blob - 3c03720d4ce022039937a283eeacc9ebbb12e2a2
> file + usr.sbin/relayd/proc.c
> --- usr.sbin/relayd/proc.c
> +++ usr.sbin/relayd/proc.c
> @@ -38,11 +38,11 @@
>  #include "log.h"
>  
>  void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
> -    char **);
> +	    char **);
>  void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
>  void	 proc_open(struct privsep *, int, int);
>  void	 proc_accept(struct privsep *, int, enum privsep_procid,
> -    unsigned int);
> +	    unsigned int);
>  void	 proc_close(struct privsep *);
>  void	 proc_shutdown(struct privsep_proc *);
>  void	 proc_sig_handler(int, short, void *);
> @@ -71,11 +71,11 @@ void
>  proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
>      int argc, char **argv)
>  {
> -	unsigned int		  proc, nargc, i, proc_i;
> +	unsigned int		 proc, nargc, i, proc_i;
>  	char			**nargv;
> -	struct privsep_proc	 *p;
> -	char			  num[32];
> -	int			  fd;
> +	struct privsep_proc	*p;
> +	char			 num[32];
> +	int			 fd;
>  
>  	/* Prepare the new process argv. */
>  	nargv = calloc(argc + 5, sizeof(char *));
> @@ -120,8 +120,8 @@ proc_exec(struct privsep *ps, struct privsep_proc *pro
>  			case 0:
>  				/* Prepare parent socket. */
>  				if (fd != PROC_PARENT_SOCK_FILENO) {
> -					if (dup2(fd, PROC_PARENT_SOCK_FILENO) ==
> -					    -1)
> +					if (dup2(fd, PROC_PARENT_SOCK_FILENO)
> +					    == -1)
>  						fatal("dup2");
>  				} else if (fcntl(fd, F_SETFD, 0) == -1)
>  					fatal("fcntl");
> @@ -158,7 +158,7 @@ proc_connect(struct privsep *ps)
>  			iev = &ps->ps_ievs[dst][inst];
>  			if (imsgbuf_init(&iev->ibuf,
>  			    ps->ps_pp->pp_pipes[dst][inst]) == -1)
> -				fatal("imsgbuf_init");
> +				fatal("%s: imsgbuf_init", __func__);
>  			imsgbuf_allow_fdpass(&iev->ibuf);
>  			event_set(&iev->ev, iev->ibuf.fd, iev->events,
>  			    iev->handler, iev->data);
> @@ -271,7 +271,7 @@ proc_accept(struct privsep *ps, int fd, enum privsep_p
>  
>  	iev = &ps->ps_ievs[dst][n];
>  	if (imsgbuf_init(&iev->ibuf, fd) == -1)
> -		fatal("imsgbuf_init");
> +		fatal("%s: imsgbuf_init", __func__);
>  	imsgbuf_allow_fdpass(&iev->ibuf);
>  	event_set(&iev->ev, iev->ibuf.fd, iev->events, iev->handler, iev->data);
>  	event_add(&iev->ev, NULL);
> @@ -616,8 +616,7 @@ proc_dispatch(int fd, short event, void *arg)
>  
>  	if (event & EV_WRITE) {
>  		if (imsgbuf_write(ibuf) == -1) {
> -			if (errno == EPIPE) {
> -				/* this pipe is dead, remove the handler */
> +			if (errno == EPIPE) {	/* Connection closed. */
>  				event_del(&iev->ev);
>  				event_loopexit(NULL);
>  				return;
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.