Re: [1/7] relayd: convert proc.c to new imsg API

On Sun, Jun 07, 2026 at 08:57:54AM +0200, Rafael Sadowski wrote:
> This is a series of commits for an imsg APIv2 rework in relayd. These
> were committed and tested individually. They can be reviewed in their
> entirety here:
> 
> gothub: https://rsadowski.gothub.org/?action=summary&headref=imsg_v2&path=relayd.git
> PR: https://codeberg.org/rsadowski/relayd/pulls/1/files
> 
> 
> commit 11d91fb058229c848c44618ab0716659f0578d8c
> Author: Rafael Sadowski <[email protected]>
> Date:   Sat Jun 6 08:43:41 2026 +0200
> 
>     relayd: convert proc.c to new imsg API
>     
>     Replace IMSG_SIZE_CHECK() + memcpy() with imsg_get_data(), which does
>     the length check and copy in one call. Use the imsg accessors
>     (imsg_get_*) instead of touching imsg.hdr directly and
>     imsgbuf_get()/imsgbuf_read() instead of imsg_get().
>     
>     Rewrite proc_forward_imsg() to use imsg_forward() per target imsgbuf
>     instead of re-composing via proc_compose_imsg(); arm the write event
>     with imsg_event_add() after each forward.
>     
>     proc_forward_imsg() never forwarded an fd, and imsg_forward() rewinds
>     the buffer internally, so multiple forwards per message keep working.
>     
>     Drop the now-unused n parameter (all callers passed -1).

Minor style issues below.
 
> diff --git a/control.c b/control.c
> index 2793461..6c29f2b 100644
> --- a/control.c
> +++ b/control.c
> @@ -361,7 +361,7 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  			break;
>  		case IMSG_CTL_SHUTDOWN:
>  		case IMSG_CTL_RELOAD:
> -			proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT, -1);
> +			proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT);
>  			break;
>  		case IMSG_CTL_POLL:
>  			proc_compose(env->sc_ps, PROC_HCE,
> @@ -385,9 +385,9 @@ control_dispatch_imsg(int fd, short event, void *arg)
>  
>  			memcpy(&verbose, imsg.data, sizeof(verbose));
>  
> -			proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT, -1);
> -			proc_forward_imsg(env->sc_ps, &imsg, PROC_HCE, -1);
> -			proc_forward_imsg(env->sc_ps, &imsg, PROC_RELAY, -1);
> +			proc_forward_imsg(env->sc_ps, &imsg, PROC_PARENT);
> +			proc_forward_imsg(env->sc_ps, &imsg, PROC_HCE);
> +			proc_forward_imsg(env->sc_ps, &imsg, PROC_RELAY);
>  
>  			memcpy(imsg.data, &verbose, sizeof(verbose));
>  			control_imsg_forward(ps, &imsg);
> diff --git a/proc.c b/proc.c
> index 361f05a..d38bebd 100644
> --- a/proc.c
> +++ b/proc.c
> @@ -599,7 +599,7 @@ proc_dispatch(int fd, short event, void *arg)
>  	struct imsgbuf		*ibuf;
>  	struct imsg		 imsg;
>  	ssize_t			 n;
> -	int			 verbose;
> +	int			 ver;
>  	const char		*title;
>  	struct privsep_fd	 pf;
>  
> @@ -607,9 +607,11 @@ proc_dispatch(int fd, short event, void *arg)
>  	ibuf = &iev->ibuf;
>  
>  	if (event & EV_READ) {
> -		if ((n = imsgbuf_read(ibuf)) == -1)
> +		switch (imsgbuf_read(ibuf)) {
> +		case -1:
>  			fatal("%s: imsgbuf_read", __func__);
> -		if (n == 0) {
> +			break;
> +		case 0:
>  			/* this pipe is dead, so remove the event handler */
>  			event_del(&iev->ev);
>  			event_loopexit(NULL);
> @@ -630,15 +632,16 @@ proc_dispatch(int fd, short event, void *arg)
>  	}
>  
>  	for (;;) {
> -		if ((n = imsg_get(ibuf, &imsg)) == -1)
> -			fatal("%s: imsg_get", __func__);
> +		if ((n = imsgbuf_get(ibuf, &imsg)) == -1)
> +			fatal("%s: imsgbuf_get", __func__);
>  		if (n == 0)
>  			break;
>  
>  #if DEBUG > 1
>  		log_debug("%s: %s %d got imsg %d peerid %d from %s %d",
>  		    __func__, title, ps->ps_instance + 1,
> -		    imsg.hdr.type, imsg.hdr.peerid, p->p_title, imsg.hdr.pid);
> +		    imsg_get_type(&imsg), imsg_get_id(&imsg),
> +		    p->p_title, imsg_get_pid(&imsg));
>  #endif
>  
>  		/*
> @@ -653,24 +656,26 @@ proc_dispatch(int fd, short event, void *arg)
>  		/*
>  		 * Generic message handling
>  		 */
> -		switch (imsg.hdr.type) {
> +		switch (imsg_get_type(&imsg)) {
>  		case IMSG_CTL_VERBOSE:
> -			IMSG_SIZE_CHECK(&imsg, &verbose);
> -			memcpy(&verbose, imsg.data, sizeof(verbose));
> -			log_setverbose(verbose);
> +			if (imsg_get_data(&imsg, &ver, sizeof(ver)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
> +			log_setverbose(ver);
>  			break;
>  		case IMSG_CTL_PROCFD:
> -			IMSG_SIZE_CHECK(&imsg, &pf);
> -			memcpy(&pf, imsg.data, sizeof(pf));
> +			if (imsg_get_data(&imsg, &pf, sizeof(pf)) == -1)
> +				fatalx("%s: imsg_get_data", __func__);
> +
>  			proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
> -			    pf.pf_instance);
> +				    pf.pf_instance);

This indent is wrong here, previous version was correct.

>  			break;
>  		default:
>  			fatalx("%s: %s %d got invalid imsg %d peerid %d "
>  			    "from %s %d",
>  			    __func__, title, ps->ps_instance + 1,
> -			    imsg.hdr.type, imsg.hdr.peerid,
> -			    p->p_title, imsg.hdr.pid);
> +			    imsg_get_type(&imsg), imsg_get_id(&imsg),
> +			    p->p_title, imsg_get_pid(&imsg));
>  		}
>  		imsg_free(&imsg);
>  	}
> @@ -790,10 +795,18 @@ proc_composev(struct privsep *ps, enum privsep_procid id,
>  
>  int
>  proc_forward_imsg(struct privsep *ps, struct imsg *imsg,
> -    enum privsep_procid id, int n)
> +    enum privsep_procid id)
>  {
> -	return (proc_compose_imsg(ps, id, n, imsg->hdr.type,
> -	    imsg->hdr.peerid, -1, imsg->data, IMSG_DATA_SIZE(imsg)));
> +	int	 m, n = -1;
> +
> +	proc_range(ps, id, &n, &m);
> +	for (; n < m; n++) {
> +		if (imsg_forward(&ps->ps_ievs[id][n].ibuf, imsg) == -1)
> +			return (-1);
> +		imsg_event_add(&ps->ps_ievs[id][n]);
> +	}
> +
> +	return (0);
>  }
>  
>  struct imsgbuf *
> diff --git a/relayd.h b/relayd.h
> index 5536b47..16fe2bf 100644
> --- a/relayd.h
> +++ b/relayd.h
> @@ -1424,7 +1424,7 @@ int	 proc_composev_imsg(struct privsep *, enum privsep_procid, int,
>  int	 proc_composev(struct privsep *, enum privsep_procid,
>  	    uint16_t, const struct iovec *, int);
>  int	 proc_forward_imsg(struct privsep *, struct imsg *,
> -	    enum privsep_procid, int);
> +	    enum privsep_procid);
>  struct imsgbuf *
>  	 proc_ibuf(struct privsep *, enum privsep_procid, int);
>  struct imsgev *
> 

Apart from that diff is OK claudio@

-- 
:wq Claudio