[PATCH v2 0/4] http: support fine-tuning curl's keepalive behavior

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@ttaylorr.com>
     
    @@ 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@ttaylorr.com>
     
      ## 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