Here's a reroll of my series to introduce new http.* knobs to control
curl's TCP keepalive behavior.
This reroll is mostly minor, with the notable differences being limited
to:
- Added error handling in the new http.c::set_long_from_env().
- Removed unnecessary casts from int -> long.
- Only set CURLOPT_TCP_KEEPCNT when compiled with a version of curl
that knows about that option to begin with (>8.9.0).
As usual, a range-diff is included below for convenience. Thanks again
for reviewing!
== Original cover letter
This short series introduces a few new http.* configuration options to
control curl's behavior around TCP keepalive packets. The details are
spelled out in the final patch, but the gist is:
- http.keepAliveIdle specifies how long in seconds to wait on an idle
connection before beginning to send keepalive packets.
- http.keepAliveInterval does the same but controls the interval
between successive keepalive packets.
- http.keepAliveCount specifies how many keepalive packets to send
before closing down the connection.
The first two commits of the series are general code clean-up of a
couple of small things I noticed while reading through the http.c code,
and the final patch implements these new options.
I couldn't think of a great way to test these new configuration
settings, and given the simplicity of the final patch I opted for no
tests there. But if someone has a good idea of how to test this
behavior, please let me know.
In either case, thanks in advance for your review!
Taylor Blau (4):
http.c: remove unnecessary casts to long
http.c: introduce `set_long_from_env()` for convenience
http.c: inline `set_curl_keepalive()`
http.c: allow custom TCP keepalive behavior via config
Documentation/config/http.adoc | 18 ++++++++
git-curl-compat.h | 7 ++++
http.c | 75 ++++++++++++++++++++++++++--------
3 files changed, 84 insertions(+), 16 deletions(-)
Range-diff against v1:
-: ---------- > 1: 204e5e18d2 http.c: remove unnecessary casts to long
1: ba22a121fa ! 2: 2e39a78e87 http.c: introduce `set_long_from_env()` for convenience
@@ Commit message
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.
+ duplication while also adding more robust error handling.
+
+ 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@xxxxxxxxxxxx>
@@ http.c: 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);
++ if (val) {
++ long tmp;
++ char *endp;
++ int saved_errno = errno;
++
++ errno = 0;
++ tmp = strtol(val, &endp, 10);
++
++ if (errno)
++ warning_errno(_("failed to parse %s"), envname);
++ else if (*endp || endp == val)
++ warning(_("failed to parse %s"), envname);
++ else
++ *var = tmp;
++
++ errno = saved_errno;
++ }
+}
+
void http_init(struct remote *remote, const char *url, int proactive_auth)
2: a05269552f = 3: cdfc9baa8d http.c: inline `set_curl_keepalive()`
3: d840415808 ! 4: 3fe62181e5 http.c: allow custom TCP keepalive behavior via config
@@ Commit message
keepalive behavior, expose configuration and environment variables which
allow setting curl's KEEPIDLE, KEEPINTVL, and KEEPCNT options.
+ Note that while KEEPIDLE and KEEPINTVL were added in curl 7.25.0,
+ KEEPCNT was added much more recently in curl 8.9.0. Per f7c094060c
+ (git-curl-compat: remove check for curl 7.25.0, 2024-10-23), both
+ KEEPIDLE and KEEPINTVL are set unconditionally. But since we may be
+ compiled with a curl that isn't as new as 8.9.0, only set KEEPCNT when
+ we have CURLOPT_TCP_KEEPCNT to begin with.
+
Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
## Documentation/config/http.adoc ##
@@ Documentation/config/http.adoc: http.lowSpeedLimit, http.lowSpeedTime::
A boolean which disables using of EPSV ftp command by curl.
This can be helpful with some "poor" ftp servers which don't
+ ## git-curl-compat.h ##
+@@
+ #define GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR 1
+ #endif
+
++/**
++ * CURLOPT_TCP_KEEPCNT was added in 8.9.0, released in July, 2024.
++ */
++#if LIBCURL_VERSION_NUM >= 0x080900
++#define GIT_CURL_HAVE_CURLOPT_TCP_KEEPCNT
++#endif
++
+ #endif
+
## http.c ##
@@ http.c: static struct {
};
@@ http.c: static int http_options(const char *var, const char *value,
}
+ if (!strcmp("http.keepaliveidle", var)) {
-+ curl_tcp_keepidle = (long)git_config_int(var, value, ctx->kvi);
++ curl_tcp_keepidle = git_config_int(var, value, ctx->kvi);
+ return 0;
+ }
+ if (!strcmp("http.keepaliveinterval", var)) {
-+ curl_tcp_keepintvl = (long)git_config_int(var, value, ctx->kvi);
++ curl_tcp_keepintvl = git_config_int(var, value, ctx->kvi);
+ return 0;
+ }
+ if (!strcmp("http.keepalivecount", var)) {
-+ curl_tcp_keepcnt = (long)git_config_int(var, value, ctx->kvi);
++ curl_tcp_keepcnt = git_config_int(var, value, ctx->kvi);
+ return 0;
+ }
+
@@ http.c: static CURL *get_curl_handle(void)
+ if (curl_tcp_keepintvl > -1)
+ curl_easy_setopt(result, CURLOPT_TCP_KEEPINTVL,
+ curl_tcp_keepintvl);
++#ifdef GIT_CURL_HAVE_CURLOPT_TCP_KEEPCNT
+ if (curl_tcp_keepcnt > -1)
+ curl_easy_setopt(result, CURLOPT_TCP_KEEPCNT, curl_tcp_keepcnt);
++#endif
+
return result;
}
base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e
--
2.49.0.4.ge59cf92f8d
