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