Re: [PATCH 3/3] http.c: allow custom TCP keepalive behavior via config

[Patrick Steinhardt <ps@pks.im>]

On Tue, Mar 18, 2025 at 06:21:41PM -0400, Taylor Blau wrote:
> curl supports a few options to control when and how often it should
> instruct the OS to send TCP keepalives, like KEEPIDLE, KEEPINTVL, and
> KEEPCNT. Until this point, there hasn't been a way for users to change
> what values are used for these options, forcing them to rely on curl's
> defaults.
> 
> But we do unconditionally enable TCP keepalives without giving users an
> ability to tweak any fine-grained parameters. Ordinarily this isn't a
> problem, particularly for users that have fast-enough connections,
> and/or are talking to a server that has generous or nonexistent
> thresholds for killing a connection it hasn't heard from in a while.
> 
> But it can present a problem when one or both of those assumptions fail.
> For instance, I can reliably get an in-progress clone to be killed from
> the remote end when cloning from some forges while using trickle to
> limit my clone's bandwidth.

Does this mean that our defaults are insufficient, as well? It's nice to
add a way to adapt those settings for the future, but ideally no user
should ever have to manually tweak them and Git should work out of the
box.

> diff --git a/http.c b/http.c
> index 526f9680f9..c13c7da530 100644
> --- a/http.c
> +++ b/http.c
> @@ -557,6 +561,19 @@ static int http_options(const char *var, const char *value,
>  		return 0;
>  	}
>  
> +	if (!strcmp("http.keepaliveidle", var)) {
> +		curl_tcp_keepidle = (long)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);
> +		return 0;
> +	}
> +	if (!strcmp("http.keepalivecount", var)) {
> +		curl_tcp_keepcnt = (long)git_config_int(var, value, ctx->kvi);
> +		return 0;
> +	}
> +
>  	/* Fall back on the default ones */
>  	return git_default_config(var, value, ctx, data);
>  }

Are the casts really necessary? The compiler shouldn't complain when
promoting from `int` to `long`.

Patrick