Re: [PATCH 1/3] http.c: introduce `set_long_from_env()` for convenience

[Elijah Newren <newren@gmail.com>]

On Tue, Mar 18, 2025 at 3:21 PM Taylor Blau <me@ttaylorr.com> 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.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  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);
> +}
> +
>  void http_init(struct remote *remote, const char *url, int proactive_auth)
>  {
> -       char *low_speed_limit;
> -       char *low_speed_time;
>         char *normalized_url;
>         struct urlmatch_config config = URLMATCH_CONFIG_INIT;
>
> @@ -1338,12 +1343,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)
>
>         set_from_env(&user_agent, "GIT_HTTP_USER_AGENT");
>
> -       low_speed_limit = getenv("GIT_HTTP_LOW_SPEED_LIMIT");
> -       if (low_speed_limit)
> -               curl_low_speed_limit = strtol(low_speed_limit, NULL, 10);
> -       low_speed_time = getenv("GIT_HTTP_LOW_SPEED_TIME");
> -       if (low_speed_time)
> -               curl_low_speed_time = strtol(low_speed_time, NULL, 10);
> +       set_long_from_env(&curl_low_speed_limit, "GIT_HTTP_LOW_SPEED_LIMIT");
> +       set_long_from_env(&curl_low_speed_time, "GIT_HTTP_LOW_SPEED_TIME");
>
>         if (curl_ssl_verify == -1)
>                 curl_ssl_verify = 1;
> --
> 2.49.0.3.gbb7a4a684c.dirty

No behavioral changes, just the use of a new convenience function;
simple enough.