Re: [PATCH 2/3] http.c: inline `set_curl_keepalive()`
- From
Elijah Newren <newren@gmail.com>
- Date
- Mar 19, 2025, 16:01 UTC
- Message-ID
- <CABPp-BEpVhP3rKXAVVZ4MUSsUZUF=YeqA23fnp0-h2S-Hk4bEw@mail.gmail.com>
- In-Reply-To
- <a05269552fc2c17519b935c3b9c279c2e231c5c5.1742336481.git.me@ttaylorr.com>
On Tue, Mar 18, 2025 at 3:21 PM Taylor Blau <me@ttaylorr.com> wrote:
Show 55 quoted lines
>
> At the end of `get_curl_handle()` we call `set_curl_keepalive()` to
> enable TCP keepalive probes on our CURL handle. `set_curl_keepalive()`
> dates back to 47ce115370 (http: use curl's tcp keepalive if available,
> 2013-10-14), which conditionally compiled different variants of
> `set_curl_keepalive()` depending on what version of curl we were
> compiled with[^1].
>
> As of f7c094060c (git-curl-compat: remove check for curl 7.25.0,
> 2024-10-23), we no longer conditionally compile `set_curl_keepalive()`
> since we no longer support pre-7.25.0 versions of curl. But the version
> of that function that we kept is really just a thin wrapper around
> setting the TCP_KEEPALIVE option, so there's no reason to keep it in its
> own function.
>
> Inline the definition of `set_curl_keepalive()` to within
> `get_curl_handle()` so that the setup of our CURL handle is
> self-contained.
>
> [1]: The details are spelled out in 47ce115370, but the gist is curl
> 7.25.0 and newer use CURLOPT_TCP_KEEPALIVE, older versions use
> CURLOPT_SOCKOPTFUNCTION with a custom callback, and older versions
> that predate even that option do nothing.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
> http.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/http.c b/http.c
> index be564fd520..526f9680f9 100644
> --- a/http.c
> +++ b/http.c
> @@ -704,10 +704,6 @@ static int has_proxy_cert_password(void)
> return 1;
> }
>
> -static void set_curl_keepalive(CURL *c)
> -{
> - curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1);
> -}
>
> /* Return 1 if redactions have been made, 0 otherwise. */
> static int redact_sensitive_header(struct strbuf *header, size_t offset)
> @@ -1242,7 +1238,7 @@ static CURL *get_curl_handle(void)
> }
> init_curl_proxy_auth(result);
>
> - set_curl_keepalive(result);
> + curl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1);
>
> return result;
> }
> --
> 2.49.0.3.gbb7a4a684c.dirtyIn contrast to the last patch, this simply dispenses with rather than adds a convenience function. That makes sense in this case, since it was only a single line of code anyway.