On Tue, Oct 22, 2024 at 12:28:25AM +0200, Omar Polo wrote:
> While we could just stick a range check and fix it, I prefer this longer
> version that splits the loop in two and simplifies the overall logic.
> dumb code is easier to follow.

Yes, much easier to follow.

> (and it's also a net negative! \o/)
> 
> oks?

ok stsp@

> diff /home/op/w/got
> commit - e2308af98f7d01e81f6173b9c264b1c21190a24a
> path + /home/op/w/got
> blob - 9f05cca64a73b00c1cebb028d16f3bd436fc1b1d
> file + gotwebd/config.c
> --- gotwebd/config.c
> +++ gotwebd/config.c
> @@ -191,33 +191,28 @@ config_setfd(struct gotwebd *env)
>  int
>  config_getfd(struct gotwebd *env, struct imsg *imsg)
>  {
> -	int match = 0, i, j;
> -	const int nfds = GOTWEB_PACK_NUM_TEMPFILES + PRIV_FDS__MAX;
> +	int i;
>  
>  	if (imsg_get_len(imsg) != 0)
>  		fatalx("%s: wrong size", __func__);
>  
> -	for (i = 0; i < nfds; i++) {
> -		if (i < PRIV_FDS__MAX && env->priv_fd[i] == -1) {
> +	for (i = 0; i < nitems(env->priv_fd); ++i) {
> +		if (env->priv_fd[i] == -1) {
>  			env->priv_fd[i] = imsg_get_fd(imsg);
>  			log_debug("%s: assigning priv_fd %d",
>  			    __func__, env->priv_fd[i]);
> -			match = 1;
> -			break;
> +			return 0;
>  		}
> +	}
>  
> -		j = i - PRIV_FDS__MAX;
> -		if (env->pack_fds[j] == -1) {
> -			env->pack_fds[j] = imsg_get_fd(imsg);
> +	for (i = 0; i < nitems(env->pack_fds); ++i) {
> +		if (env->pack_fds[i] == -1) {
> +			env->pack_fds[i] = imsg_get_fd(imsg);
>  			log_debug("%s: assigning pack_fd %d",
> -			    __func__, env->pack_fds[j]);
> -			match = 1;
> -			break;
> +			    __func__, env->pack_fds[i]);
> +			return 0;
>  		}
>  	}
>  
> -	if (match)
> -		return 0;
> -	else
> -		return 1;
> +	return 1;
>  }
> 
>