From: Rafael Sadowski <rafael@sizeofvoid.org>
Subject: Re: [7/7+1] relayd: ibuf_get_string / ibuf_get_data
To: tech@openbsd.org
Date: Tue, 9 Jun 2026 21:17:08 +0200

On Tue Jun 09, 2026 at 08:55:41PM +0200, Claudio Jeker wrote:
> Looks good to me. OK claudio@
> Is there a reason why you implemented get_string instead of just using
> ibuf_get_string()? To me it looks like both return the same result.

Because I just saw the "str = strndup(ibuf_data(buf), len);" and not the
"buf->rpos += len;" For sure I'll use ibuf_get_string form libutil instead.

> 
> I wonder how many other reykd use get_string and get_data as well.

I'll have a look at it. But first, there is more in relayd/imsg space.

Rafael

diff --git a/config.c b/config.c
index 7ca37d5..c4a12e4 100644
--- a/config.c
+++ b/config.c
@@ -361,8 +361,8 @@ config_gettable(struct relayd *env, struct imsg *imsg)
 		return (-1);
 	}
 	if ((sb = ibuf_size(&ibuf)) > 0) {
-		if ((tb->sendbuf = get_string(ibuf_data(&ibuf), sb)) == NULL) {
-			log_warn("%s: get_string", __func__);
+		if ((tb->sendbuf = ibuf_get_string(&ibuf, sb)) == NULL) {
+			log_warn("%s: ibuf_get_string", __func__);
 			free(tb);
 			return (-1);
 		}
@@ -732,9 +732,9 @@ config_getproto(struct relayd *env, struct imsg *imsg)
 	}
 	if ((s = ibuf_size(&ibuf)) > 0) {
 		proto->style = NULL;
-		if ((proto->style = get_string(ibuf_data(&ibuf), s - 1))
+		if ((proto->style = ibuf_get_string(&ibuf, s - 1))
 		    == NULL) {
-			log_warn("%s: get_string", __func__);
+			log_warn("%s: ibuf_get_string", __func__);
 			free(proto);
 			return (-1);
 		}
@@ -790,11 +790,7 @@ config_getrule(struct relayd *env, struct imsg *imsg)
 		/* Also accept "empty" 0-length strings */		\
 		if (ibuf_size(&ibuf) < (size_t)len ||			\
 		    (rule->rule_kv[_n].kv_##_f =			\
-		    get_string(ibuf_data(&ibuf), len)) == NULL) {	\
-			free(rule);					\
-			return (-1);					\
-		}							\
-		if (len > 0 && ibuf_skip(&ibuf, len) == -1) {		\
+		    ibuf_get_string(&ibuf, len)) == NULL) {		\
 			free(rule);					\
 			return (-1);					\
 		}							\
@@ -1104,8 +1100,7 @@ config_getrelay(struct relayd *env, struct imsg *imsg)
 		goto fail;
 	}
 	if (s > 0) {
-		if ((rlay->rl_tls_cakey = get_data(ibuf_data(&ibuf), s))
-		    == NULL)
+		if ((rlay->rl_tls_cakey = get_data(&ibuf, s)) == NULL)
 			goto fail;
 	}
 
diff --git a/relayd.c b/relayd.c
index b678103..201144c 100644
--- a/relayd.c
+++ b/relayd.c
@@ -437,8 +437,8 @@ parent_dispatch_pfe(int fd, struct privsep_proc *p, struct imsg *imsg)
 	case IMSG_CTL_RELOAD:
 		if (imsg_get_ibuf(imsg, &ibuf) != -1 &&
 		    (s = ibuf_size(&ibuf)) > 0) {
-			if ((str = get_string(ibuf_data(&ibuf), s)) == NULL) {
-				log_warn("%s: get_string", __func__);
+			if ((str = ibuf_get_string(&ibuf, s)) == NULL) {
+				log_warn("%s: ibuf_get_string", __func__);
 				return (-1);
 			}
 		}
@@ -1806,28 +1806,21 @@ socket_rlimit(int maxfd)
 		fatal("%s: failed to set resource limit", __func__);
 }
 
-char *
-get_string(u_int8_t *ptr, size_t len)
-{
-	size_t	 i;
-
-	for (i = 0; i < len; i++)
-		if (!(isprint((unsigned char)ptr[i]) ||
-		    isspace((unsigned char)ptr[i])))
-			break;
-
-	return strndup(ptr, i);
-}
-
 void *
-get_data(u_int8_t *ptr, size_t len)
+get_data(struct ibuf *ibuf, size_t len)
 {
 	u_int8_t	*data;
 
+	if (len == 0)
+		return (NULL);
+
 	if ((data = malloc(len)) == NULL)
 		return (NULL);
-	memcpy(data, ptr, len);
 
+	if (ibuf_get(ibuf, data, len) == -1) {
+		free(data);
+		return (NULL);
+	}
 	return (data);
 }
 
diff --git a/relayd.h b/relayd.h
index bd29888..ac771fb 100644
--- a/relayd.h
+++ b/relayd.h
@@ -1336,8 +1336,7 @@ void		 imsg_event_add(struct imsgev *);
 int		 imsg_compose_event(struct imsgev *, u_int16_t, u_int32_t,
 		    pid_t, int, void *, u_int16_t);
 void		 socket_rlimit(int);
-char		*get_string(u_int8_t *, size_t);
-void		*get_data(u_int8_t *, size_t);
+void		*get_data(struct ibuf *, size_t);
 int		 sockaddr_cmp(struct sockaddr *, struct sockaddr *, int);
 struct in6_addr *prefixlen2mask6(u_int8_t, u_int32_t *);
 u_int32_t	 prefixlen2mask(u_int8_t);