Download raw body.
relayd: use imsg_get_type and imsg_get_type
On Tue Jul 28, 2026 at 09:37:30PM +0200, Theo Buehler wrote:
> On Sat, Jul 25, 2026 at 07:23:56PM +0200, Rafael Sadowski wrote:
> > OK?
> >
> > commit 0b7c23a563ac96506f78c660a431a3e3d6f2d020
> > Author: Rafael Sadowski <rafael@sizeofvoid.org>
> > 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.
I'll commit it in two different commits.
>
> >
> > 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().
Yes, it's certainly better for debugging.
>
> If that's intentional deduplication, I guess it's fine, it's just a bit
> confusing for the casual reader.
>
That wasn't the intention. I can see how it might have been confusing to review.
diff --git a/hce.c b/hce.c
index 82f3003..2d63d26 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)
+ fatalx("%s: imsg_get_data", __func__);
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)
+ fatalx("%s: imsg_get_data", __func__);
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)
+ fatalx("%s: imsg_get_data", __func__);
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)
+ fatalx("%s: imsg_get_data", __func__);
if ((table = table_find(env, id)) == NULL)
fatalx("%s: desynchronized", __func__);
table->conf.flags &= ~(F_DISABLE);
@@ -345,7 +349,7 @@ hce_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
switch (imsg_get_type(imsg)) {
case IMSG_SCRIPT:
if (imsg_get_data(imsg, &scr, sizeof(scr)) == -1)
- return (-1);
+ fatalx("%s: imsg_get_data", __func__);
script_done(env, &scr);
break;
case IMSG_CFG_TABLE:
diff --git a/relay.c b/relay.c
index 412ae74..70f66c3 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)
+ fatalx("%s: imsg_get_data", __func__);
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)
+ fatalx("%s: imsg_get_data", __func__);
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)
+ fatalx("%s: imsg_get_data", __func__);
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)
+ fatalx("%s: imsg_get_data", __func__);
if ((table = table_find(env, id)) == NULL)
fatalx("%s: desynchronized", __func__);
table->conf.flags &= ~(F_DISABLE);
@@ -1908,7 +1912,7 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
break;
case IMSG_HOST_STATUS:
if (imsg_get_data(imsg, &st, sizeof(st)) == -1)
- return (-1);
+ fatalx("%s: imsg_get_data", __func__);
if ((host = host_find(env, st.id)) == NULL)
fatalx("%s: invalid host id", __func__);
if (host->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)
+ fatalx("%s: imsg_get_data", __func__);
if ((con = session_find(env, cnl.id)) == NULL ||
con->se_cnl == NULL) {
log_debug("%s: session %d: expired",
@@ -1954,7 +1959,7 @@ relay_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
break;
case IMSG_CTL_SESSION:
if (imsg_get_data(imsg, &cid, sizeof(cid)) == -1)
- return (-1);
+ fatalx("%s: imsg_get_data", __func__);
TAILQ_FOREACH(rlay, env->sc_relays, rl_entry) {
SPLAY_FOREACH(con, session_tree,
&rlay->rl_sessions) {
@@ -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)
+ fatalx("%s: imsg_get_data", __func__);
if ((con = session_find(env, id)) == NULL) {
log_debug("%s: session %d: expired",
__func__, id);
@@ -2058,7 +2064,7 @@ relay_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
case IMSG_TLSTICKET_REKEY:
if (imsg_get_data(imsg, &env->sc_ticket,
sizeof(env->sc_ticket)) == -1)
- return (-1);
+ fatalx("%s: imsg_get_data", __func__);
TAILQ_FOREACH(rlay, env->sc_relays, rl_entry) {
if (rlay->rl_conf.flags & F_TLS)
tls_config_add_ticket_key(rlay->rl_tls_cfg,
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;
relayd: use imsg_get_type and imsg_get_type