From: Martijn van Duren Subject: Re: {relayd,httpd}/proc.c: sync trivial bits To: Rafael Sadowski Cc: tech@openbsd.org Date: Wed, 8 Jul 2026 08:30:59 +0200 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. 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;