Re: relayd: use imsg_get_type and imsg_get_type

On Sat, Jul 25, 2026 at 07:23:56PM +0200, Rafael Sadowski wrote:
> OK?
> 
> commit 0b7c23a563ac96506f78c660a431a3e3d6f2d020
> Author: Rafael Sadowski <[email protected]>
> Date:   Sat Jul 25 19:20:21 2026 +0200
> 
>     relayd: use imsg_get_type and imsg_get_type

Feels like two commits (one for relayd.c and one for the other two)
and the second one concerns imsg_get_data, not imsg_get_type.

> 
> diff --git a/hce.c b/hce.c
> index 82f3003..05c1555 100644
> --- a/hce.c
> +++ b/hce.c
> @@ -291,7 +291,8 @@ hce_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  
>  	switch (imsg_get_type(imsg)) {
>  	case IMSG_HOST_DISABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);

It might be preferable to fatal explicitly rather than having readers go
look at the reykian proc.c where it falls through to another fatalx()
in proc_dispatch().

If that's intentional deduplication, I guess it's fine, it's just a bit
confusing for the casual reader.

>  		if ((host = host_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		host->flags |= F_DISABLE;
> @@ -301,7 +302,8 @@ hce_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  		host->he = HCE_NONE;
>  		break;
>  	case IMSG_HOST_ENABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((host = host_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		host->flags &= ~(F_DISABLE);
> @@ -309,7 +311,8 @@ hce_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  		host->he = HCE_NONE;
>  		break;
>  	case IMSG_TABLE_DISABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((table = table_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		table->conf.flags |= F_DISABLE;
> @@ -317,7 +320,8 @@ hce_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  			host->up = HOST_UNKNOWN;
>  		break;
>  	case IMSG_TABLE_ENABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((table = table_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		table->conf.flags &= ~(F_DISABLE);
> diff --git a/relay.c b/relay.c
> index 412ae74..58e57b7 100644
> --- a/relay.c
> +++ b/relay.c
> @@ -1870,7 +1870,8 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  
>  	switch (imsg_get_type(imsg)) {
>  	case IMSG_HOST_DISABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((host = host_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		if ((table = table_find(env, host->conf.tableid)) ==
> @@ -1882,14 +1883,16 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  		host->up = HOST_UNKNOWN;
>  		break;
>  	case IMSG_HOST_ENABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((host = host_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		host->flags &= ~(F_DISABLE);
>  		host->up = HOST_UNKNOWN;
>  		break;
>  	case IMSG_TABLE_DISABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((table = table_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		table->conf.flags |= F_DISABLE;
> @@ -1898,7 +1901,8 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  			host->up = HOST_UNKNOWN;
>  		break;
>  	case IMSG_TABLE_ENABLE:
> -		memcpy(&id, imsg->data, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((table = table_find(env, id)) == NULL)
>  			fatalx("%s: desynchronized", __func__);
>  		table->conf.flags &= ~(F_DISABLE);
> @@ -1939,7 +1943,8 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  		host->up = st.up;
>  		break;
>  	case IMSG_NATLOOK:
> -		bcopy(imsg->data, &cnl, sizeof(cnl));
> +		if (imsg_get_data(imsg, &cnl, sizeof(cnl)) == -1)
> +			return (-1);
>  		if ((con = session_find(env, cnl.id)) == NULL ||
>  		    con->se_cnl == NULL) {
>  			log_debug("%s: session %d: expired",
> @@ -2001,7 +2006,8 @@ relay_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
>  
>  	switch (imsg_get_type(imsg)) {
>  	case IMSG_BINDANY:
> -		bcopy(imsg->data, &id, sizeof(id));
> +		if (imsg_get_data(imsg, &id, sizeof(id)) == -1)
> +			return (-1);
>  		if ((con = session_find(env, id)) == NULL) {
>  			log_debug("%s: session %d: expired",
>  			    __func__, id);
> diff --git a/relayd.c b/relayd.c
> index 7b8af3b..b174fc8 100644
> --- a/relayd.c
> +++ b/relayd.c
> @@ -407,7 +407,7 @@ parent_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
>  	char			*str = NULL;
>  	size_t			 s;
>  
> -	switch (imsg->hdr.type) {
> +	switch (imsg_get_type(imsg)) {
>  	case IMSG_DEMOTE:
>  		if (imsg_get_data(imsg, &demote, sizeof(demote)) == -1) {
>  			log_warn("%s: imsg_get_data", __func__);
> @@ -469,7 +469,7 @@ parent_dispatch_hce(int fd, struct privsep_proc *p, struct imsg *imsg)
>  	struct relayd		*env = ps->ps_env;
>  	struct ctl_script	 scr;
>  
> -	switch (imsg->hdr.type) {
> +	switch (imsg_get_type(imsg)) {
>  	case IMSG_SCRIPT:
>  		if (imsg_get_data(imsg, &scr, sizeof(scr)) == -1) {
>  			log_warn("%s: imsg_get_data", __func__);
> @@ -498,7 +498,7 @@ parent_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
>  	struct ctl_bindany	 bnd;
>  	int			 s;
>  
> -	switch (imsg->hdr.type) {
> +	switch (imsg_get_type(imsg)) {
>  	case IMSG_BINDANY:
>  		if (imsg_get_data(imsg, &bnd, sizeof(bnd)) == -1) {
>  			log_warn("%s: imsg_get_data", __func__);
> @@ -537,7 +537,7 @@ parent_dispatch_ca(int fd, struct privsep_proc *p, struct imsg *imsg)
>  	struct privsep		*ps = p->p_ps;
>  	struct relayd		*env = ps->ps_env;
>  
> -	switch (imsg->hdr.type) {
> +	switch (imsg_get_type(imsg)) {
>  	case IMSG_CFG_DONE:
>  		parent_configure_done(env);
>  		break;
>