Thread (21 messages) 21 messages, 4 authors, 2025-04-01

Re: [PATCH 2/3] http.c: inline `set_curl_keepalive()`

From: Elijah Newren <hidden>
Date: 2025-03-19 16:01:34

On Tue, Mar 18, 2025 at 3:21 PM Taylor Blau [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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.dirty
In 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help