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

Rafael Sadowski <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
On Wed Jul 08, 2026 at 08:30:59AM +0200, Martijn van Duren wrote:
> On 7/2/26 19:18, Rafael Sadowski wrote:
> > 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@
> >>
> Here's a diff based around knfmt.

Everything looks good, ok rsadowski

> diff /usr/src
> path + /usr/src
> commit - 68775dfeacaa4a53a91d89f7deb8a8d2a4e03739
> blob - 46eece383197b35991f1750803d346bfadda0471
> file + usr.sbin/httpd/proc.c
> --- usr.sbin/httpd/proc.c
> +++ usr.sbin/httpd/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 *));
> @@ -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;
> @@ -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(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 - 68775dfeacaa4a53a91d89f7deb8a8d2a4e03739
> blob - 121fd742b2ea03a28dd19e35f8692ec83ff6053a
> file + usr.sbin/relayd/proc.c
> --- usr.sbin/relayd/proc.c
> +++ usr.sbin/relayd/proc.c
> @@ -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.