From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 20 Mar 2025 05:24:47 GMT
Subject: Re: [PATCH v2 2/4] http.c: introduce `set_long_from_env()` for convenience
Message-ID: <Z9umnwmhZTZ82SnU@pks.im>
In-Reply-To: <2e39a78e87edaf8f9842e510d05047dce647f4af.1742423021.git.me@ttaylorr.com>

On Wed, Mar 19, 2025 at 06:23:50PM -0400, Taylor Blau wrote:
> diff --git a/http.c b/http.c
> index 0cbcb079b2..17b676a1d5 100644
> --- a/http.c
> +++ b/http.c
> @@ -1256,10 +1256,30 @@ static void set_from_env(char **var, const char *envname)
>  	}
>  }
>  
> +static void set_long_from_env(long *var, const char *envname)
> +{
> +	const char *val = getenv(envname);
> +	if (val) {
> +		long tmp;
> +		char *endp;
> +		int saved_errno = errno;
> +
> +		errno = 0;
> +		tmp = strtol(val, &endp, 10);
> +
> +		if (errno)
> +			warning_errno(_("failed to parse %s"), envname);
> +		else if (*endp || endp == val)
> +			warning(_("failed to parse %s"), envname);
> +		else
> +			*var = tmp;
> +
> +		errno = saved_errno;
> +	}
> +}

The `saved_errno` dance feels a bit unnecessary, but other than that I'm
okay with this approach of only printing a warning instead of dying
right away.

The other patches look good to me, too, thanks!

Patrick