Re: [PATCH 1/3] http.c: introduce `set_long_from_env()` for convenience
- From
Elijah Newren <newren@gmail.com>
- Date
- Mar 19, 2025, 16:00 UTC
- Message-ID
- <CABPp-BGcsVAbaAj628ACGb=cgMaUBH0LO316tj9pJsj6=WrH8w@mail.gmail.com>
- In-Reply-To
- <ba22a121fa699e490de00eba988552b6c10fe2fd.1742336481.git.me@ttaylorr.com>
On Tue, Mar 18, 2025 at 3:21 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 63 quoted lines
>
> 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.dirtyNo behavioral changes, just the use of a new convenience function; simple enough.