On Mon, Oct 14, 2013 at 05:27:39AM +0000, Eric Wong wrote:
> Daniel Stenberg <dan...@haxx.se> wrote:
> > On Sat, 12 Oct 2013, Eric Wong wrote:
> >
> > >This is a follow up to commit
> > >e47a8583a20256851e7fc882233e3bd5bf33dc6e (enable SO_KEEPALIVE for
> > >connected TCP sockets).
> >
> > Just keep in mind that TCP keep-alive is enabled in awkwardly many
> > different ways on different systems and this patch only supports one
> > of them. Feel free to take inspiration from libcurl's source code
> > for doing this. See:
> >
> > https://github.com/bagder/curl/blob/master/lib/connect.c#L108
>
> Thanks. I think the Linux-specific TCP_KEEP* knobs are overkill for git.
> (since this is mainly for non-interactive users, I went at least a day
> before realizing the process was stuck on my machine).
> I cannot comment on the knobs for other OSes.
I don't think we should get into having a big compatibility layer that
just reproduces what is in curl.
But is there any reason not to use CURLOPT_TCP_KEEPALIVE when it is
available, falling back to CURLOPT_SOCKOPTFUNCTION, and then finally to
nothing? That lets people on modern curl benefit from curl's more
portable code, without punishing people on older versions.
I.e., something like:
---
diff --git a/http.c b/http.c
index 5834c9b..e221efb 100644
--- a/http.c
+++ b/http.c
@@ -254,36 +254,54 @@ static int sockopt_callback(void *client, curl_socket_t
fd, curlsocktype type)
if (!cert_auth.password) {
cert_auth.protocol = xstrdup("cert");
cert_auth.username = xstrdup("");
cert_auth.path = xstrdup(ssl_cert);
credential_fill(&cert_auth);
}
return 1;
}
-/* curl 7.25.0 has CURLOPT_TCP_KEEPALIVE, too, but we support older curl */
+#if LIBCURL_VERSION_NUM >= 0x071900
+static void set_curl_keepalive(CURL *c)
+{
+ curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1);
+}
+
+#elif LIBCURL_VERSION_NUM >= 0x071000
static int sockopt_callback(void *client, curl_socket_t fd, curlsocktype type)
{
int ka = 1;
int rc;
socklen_t len = (socklen_t)sizeof(ka);
if (type != CURLSOCKTYPE_IPCXN)
return 0;
rc = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, (void *)&ka, len);
if (rc < 0)
warning("unable to set SO_KEEPALIVE on socket %s",
strerror(errno));
return 0; /* CURL_SOCKOPT_OK only exists since curl 7.21.5 */
}
+static void set_curl_keepalive(CURL *c)
+{
+ curl_easy_setopt(c, CURLOPT_SOCKOPTFUNCTION, sockopt_callback);
+}
+
+#else
+static void set_curl_keepalive(CURL *c)
+{
+ /* not supported on older curl versions */
+}
+#endif
+
static CURL *get_curl_handle(void)
{
CURL *result = curl_easy_init();
if (!curl_ssl_verify) {
curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0);
curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0);
} else {
/* Verify authenticity of the peer's certificate */
@@ -344,21 +362,19 @@ static CURL *get_curl_handle(void)
if (curl_ssl_try)
curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);
#endif
if (curl_http_proxy) {
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
}
-#if LIBCURL_VERSION_NUM >= 0x071000
- curl_easy_setopt(result, CURLOPT_SOCKOPTFUNCTION, sockopt_callback);
-#endif
+ set_curl_keepalive(result);
return result;
}
static void set_from_env(const char **var, const char *envname)
{
const char *val = getenv(envname);
if (val)
*var = val;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
