From 820d81ca445aa56f839540bca9d36e1be8c2efc2 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 1b030d96f4..f936078491 100644 --- a/http-push.c +++ b/http-push.c @@ -194,7 +194,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 d5396a3ce2..5798d6151a 100644 --- a/http.c +++ b/http.c @@ -731,7 +731,7 @@ static int has_proxy_cert_password(void) static void set_curl_keepalive(CURL *c) { - curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1); + curl_easy_setopt(c, CURLOPT_TCP_KEEPALIVE, 1L); } /* Return 1 if redactions have been made, 0 otherwise. */ @@ -1032,13 +1032,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) { @@ -1141,7 +1141,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 @@ -1175,7 +1175,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); diff --git a/remote-curl.c b/remote-curl.c index 1273507a96..7690d26fc0 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 b0095e932f30d7984d450bd2b5ba21b09ffb6163 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 6c8f84e836..b42986be34 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1418,7 +1418,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; @@ -1431,8 +1431,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 018cbc47d7a3e7e4b60be59ffe1a772bdcb6a9b5 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 | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/http.c b/http.c index 5798d6151a..0cec685b8d 100644 --- a/http.c +++ b/http.c @@ -1142,7 +1142,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 { @@ -1217,18 +1217,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);