On Tue, Mar 18, 2025 at 06:21:34PM -0400, Taylor Blau wrote:
> In 7059cd99fc (http_init(): Fix config file parsing, 2009-03-09), http.c
> gained a new "set_from_env()" function as a convenience function around
> conditionally assigning an environment variable to some variable if and
> only if the environment variable was set to begin with.
>
> But prior to 7059cd99fc, there were two spots which need to first
> strtol() whatever is set in the environment before assigning it to a
> long pointer. Both instances stored the result of getenv() in a
> temporary variable, and conditionally strtol() it depending on whether
> or not getenv() returned NULL.
>
> Replace those two instances with a new cousin of 'set_from_env()' called
> 'set_long_from_env()', which does what its name suggests. This allows us
> to remove the temporary variables and clean up some minor code
> duplication. More importantly, however, it prepares us for a future
> commit which will introduce more instances of assigning an environment
> variable to a long.
Okay, makes sense.
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> http.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/http.c b/http.c
> index 0c9a872809..be564fd520 100644
> --- a/http.c
> +++ b/http.c
> @@ -1256,10 +1256,15 @@ 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)
> + *var = strtol(val, NULL, 10);
> +}
Hm. We don't perform any error checking at all for whether or not the
value of the environment variable is a valid integer. This isn't a new
issue introduced by your patch, but now that we have a central place
where it's being parsed I wonder whether we should be checking for
errors?
Patrick
