From 6f11c42e8edc5cf7d65156c9dd68e720f1b92229 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 4 Jun 2025 16:55:13 -0400 Subject: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt() The curl documentation specifies that curl_easy_setopt() takes either: ...a long, a function pointer, an object pointer or a curl_off_t, depending on what the specific option expects. But when we pass an integer constant like "0", it will by default be a regular non-long int. This has always been wrong, but seemed to work in practice (I didn't dig into curl's implementation to see whether this might actually be triggering undefined behavior, but it seems likely and regardless we should do what the docs say). This is especially important since curl has a type-checking macro that causes building against curl 8.14 to produce many warnings. The specific commit is due to their 79b4e56b3 (typecheck-gcc.h: fix the typechecks, 2025-04-22). Curiously, it does only seem to trigger when compiled with -O2 for me. We can fix it by just marking the constants with a long "L". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http-push.c | 2 +- http.c | 14 +++++++------- remote-curl.c | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/http-push.c b/http-push.c index f9e67cabd4..591e46ab26 100644 --- a/http-push.c +++ b/http-push.c @@ -195,7 +195,7 @@ static char *xml_entities(const char *s) static void curl_setup_http_get(CURL *curl, const char *url, const char *custom_req) { - curl_easy_setopt(curl, CURLOPT_HTTPGET, 1); + curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L); curl_easy_setopt(curl, CURLOPT_URL, url); curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req); curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, fwrite_null); diff --git a/http.c b/http.c index 3c029cf894..cce2ea7287 100644 --- a/http.c +++ b/http.c @@ -1019,13 +1019,13 @@ static CURL *get_curl_handle(void) die("curl_easy_init failed"); if (!curl_ssl_verify) { - curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0); - curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0); + curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0L); + curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0L); } else { /* Verify authenticity of the peer's certificate */ - curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 1); + curl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 1L); /* The name in the cert must match whom we tried to connect */ - curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2); + curl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2L); } if (curl_http_version) { @@ -1117,7 +1117,7 @@ static CURL *get_curl_handle(void) curl_low_speed_time); } - curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20); + curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20L); curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL); #ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR @@ -1151,7 +1151,7 @@ static CURL *get_curl_handle(void) user_agent ? user_agent : git_user_agent()); if (curl_ftp_no_epsv) - curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0); + curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0L); if (curl_ssl_try) curl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY); @@ -1254,7 +1254,7 @@ static CURL *get_curl_handle(void) } init_curl_proxy_auth(result); - curl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1); + curl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1L); if (curl_tcp_keepidle > -1) curl_easy_setopt(result, CURLOPT_TCP_KEEPIDLE, diff --git a/remote-curl.c b/remote-curl.c index 590b228f67..6183772191 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -877,12 +877,12 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results) headers = curl_slist_append(headers, rpc->hdr_content_type); headers = curl_slist_append(headers, rpc->hdr_accept); - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); - curl_easy_setopt(slot->curl, CURLOPT_POST, 1); + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L); + curl_easy_setopt(slot->curl, CURLOPT_POST, 1L); curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url); curl_easy_setopt(slot->curl, CURLOPT_ENCODING, NULL); curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, "0000"); - curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4); + curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4L); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &buf); From 30325e23ba0d40567cc4ef78e4ba0c3776ef0c06 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 4 Jun 2025 16:55:52 -0400 Subject: [PATCH 2/3] curl: fix integer variable typechecks with curl_easy_setopt() As discussed in the previous commit, we should be passing long integers, not regular ones, to curl_easy_setopt(), and compiling against curl 8.14 loudly complains if we don't. That patch fixed integer constants by adding an "L". This one deals with actual variables. Arguably these variables could just be declared as "long" in the first place. But it's actually kind of awkward due to other code which uses them: - port is conceptually a short, and we even call htons() on it (though weirdly it is defined as a regular int). - ssl_verify is conceptually a bool, and we assign to it from git_config_bool(). So I think we could probably switch these out for longs without hurting anything, but it just feels a bit weird. Doubly so because if you don't set USE_CURL_FOR_IMAP_SEND set, then the current types are fine! So let's just cast these to longs in the curl calls, which makes what's going on obvious. There aren't that many spots to modify (and as you can see from the context, we already have some similar casts). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- imap-send.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/imap-send.c b/imap-send.c index 27dc033c7f..2e812f5a6e 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1420,7 +1420,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred) curl_easy_setopt(curl, CURLOPT_URL, path.buf); strbuf_release(&path); - curl_easy_setopt(curl, CURLOPT_PORT, srvc->port); + curl_easy_setopt(curl, CURLOPT_PORT, (long)srvc->port); if (srvc->auth_method) { struct strbuf auth = STRBUF_INIT; @@ -1433,8 +1433,8 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred) if (!srvc->use_ssl) curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_TRY); - curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, srvc->ssl_verify); - curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, srvc->ssl_verify); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, (long)srvc->ssl_verify); + curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, (long)srvc->ssl_verify); curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer); From 4558c8f84b2f8d3ba1483727bcb49935ae8ff595 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 4 Jun 2025 16:56:22 -0400 Subject: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt() As with the previous two commits, we should be passing long integers, not regular ones, to curl_easy_setopt(), and compiling against curl 8.14 loudly complains if we don't. This patch catches the remaining cases, which are ones where we pass curl's own symbolic constants. We'll cast them to long manually in each call. It seems kind of weird to me that curl doesn't define these constants as longs, since the point of them is to pass to curl_easy_setopt(). But in the curl documentation and examples, they clearly show casting them as part of the setopt calls. It may be that there is some reason not to push the type into the macro, like backwards compatibility. I didn't dig, as it doesn't really matter: we have to follow what existing curl versions ask for anyway. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/http.c b/http.c index cce2ea7287..ecbc47ea4b 100644 --- a/http.c +++ b/http.c @@ -1057,7 +1057,7 @@ static CURL *get_curl_handle(void) if (http_ssl_backend && !strcmp("schannel", http_ssl_backend) && !http_schannel_check_revoke) { - curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE); + curl_easy_setopt(result, CURLOPT_SSL_OPTIONS, (long)CURLSSLOPT_NO_REVOKE); } if (http_proactive_auth != PROACTIVE_AUTH_NONE) @@ -1118,7 +1118,7 @@ static CURL *get_curl_handle(void) } curl_easy_setopt(result, CURLOPT_MAXREDIRS, 20L); - curl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL); + curl_easy_setopt(result, CURLOPT_POSTREDIR, (long)CURL_REDIR_POST_ALL); #ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR { @@ -1193,18 +1193,18 @@ static CURL *get_curl_handle(void) if (starts_with(curl_http_proxy, "socks5h")) curl_easy_setopt(result, - CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5_HOSTNAME); + CURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS5_HOSTNAME); else if (starts_with(curl_http_proxy, "socks5")) curl_easy_setopt(result, - CURLOPT_PROXYTYPE, CURLPROXY_SOCKS5); + CURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS5); else if (starts_with(curl_http_proxy, "socks4a")) curl_easy_setopt(result, - CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4A); + CURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS4A); else if (starts_with(curl_http_proxy, "socks")) curl_easy_setopt(result, - CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4); + CURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS4); else if (starts_with(curl_http_proxy, "https")) { - curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS); + curl_easy_setopt(result, CURLOPT_PROXYTYPE, (long)CURLPROXY_HTTPS); if (http_proxy_ssl_cert) curl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);