On 2023/11/16 22:56:59 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> On Thu, Nov 16, 2023 at 10:49:06PM +0100, Omar Polo wrote:
> > On 2023/11/16 22:03:16 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > > On Wed, Nov 15, 2023 at 07:46:57PM +0100, Omar Polo wrote:
> > > > +#define IMSG_SIZE_CHECK(imsg, p) do {					\
> > > > +	if (IMSG_DATA_SIZE(imsg) < sizeof(*p))				\
> > > 
> > > Could this check even be != sizeof(*p)?
> > > As far as I can see all messages used by gotwebd are of a fixed size.
> > 
> > IIRC this define is used by other daemons as well and I'd like not to
> > change its behaviour here.
> > 
> > Now, actually I'd prefer to remove completely its usage.  I always get
> > confused about whether it takes a pointer or the variable directly, and
> > I've fixed at least a few wrong usage (one was fixed in the diff
> > itself.)
> 
> Sure. I have no concerns about removing these macros. They were never
> needed in other code we have that processes such messages.

and this is the last diff, mostly since i had it still in my tree.  ok?

-----------------------------------------------
commit f4215325bdd421885152307b5b785f0cef149ee0 (main)
from: Omar Polo <op@omarpolo.com>
date: Fri Nov 17 09:28:55 2023 UTC
 
 gotwebd: inline and remove IMSG_SIZE_CHECK()
 
diff 1632f50aca5cd94ed681c20fc18c2b8ab4857b9c f4215325bdd421885152307b5b785f0cef149ee0
commit - 1632f50aca5cd94ed681c20fc18c2b8ab4857b9c
commit + f4215325bdd421885152307b5b785f0cef149ee0
blob - b1108c45d826da11c2d77f4d5c684f9c629ea7c8
blob + bde889cb381f5d8e90f44b27a11986e29f2f27d2
--- gotwebd/config.c
+++ gotwebd/config.c
@@ -82,7 +82,8 @@ config_getserver(struct gotwebd *env, struct imsg *ims
 	if (srv == NULL)
 		fatalx("%s: calloc", __func__);
 
-	IMSG_SIZE_CHECK(imsg, srv);
+	if (IMSG_DATA_SIZE(imsg) != sizeof(*srv))
+		fatalx("%s: wrong size", __func__);
 
 	memcpy(srv, p, sizeof(*srv));
 
@@ -119,7 +120,9 @@ config_getsock(struct gotwebd *env, struct imsg *imsg)
 	uint8_t *p = imsg->data;
 	int i;
 
-	IMSG_SIZE_CHECK(imsg, &sock_conf);
+	if (IMSG_DATA_SIZE(imsg) != sizeof(sock_conf))
+		fatalx("%s: wrong size", __func__);
+
 	memcpy(&sock_conf, p, sizeof(sock_conf));
 
 	if (IMSG_DATA_SIZE(imsg) != sizeof(sock_conf)) {
@@ -194,7 +197,9 @@ config_getfd(struct gotwebd *env, struct imsg *imsg)
 	uint8_t *p = imsg->data;
 	int sock_id, match = 0, i;
 
-	IMSG_SIZE_CHECK(imsg, &sock_id);
+	if (IMSG_DATA_SIZE(imsg) != sizeof(sock_id))
+		fatalx("%s: wrong size", __func__);
+
 	memcpy(&sock_id, p, sizeof(sock_id));
 
 	TAILQ_FOREACH(sock, &env->sockets, entry) {
blob - 71b34a0258f2a201a28b0745032caf69c7fcd5e6
blob + 8005d4db4e8e1da67b226c26b8e1640b5c42645d
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -137,11 +137,6 @@ struct imsgev {
 	short			 events;
 };
 
-#define IMSG_SIZE_CHECK(imsg, p) do {					\
-	if (IMSG_DATA_SIZE(imsg) < sizeof(*p))				\
-		fatalx("bad length imsg received (%s)",	#p);		\
-} while (0)
-
 #define IMSG_DATA_SIZE(imsg)	((imsg)->hdr.len - IMSG_HEADER_SIZE)
 
 struct env_val {