From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: get rid of proc.[ch]
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 17 Nov 2023 10:38:36 +0100

On Fri, Nov 17, 2023 at 10:34:50AM +0100, Omar Polo wrote:
> 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?

Oh, nice. Yes.

> -----------------------------------------------
> 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 {
> 
> 
>