{"thread":{"id":"63585","subject":"[PATCH 0/3] silencing warnings with curl 8.14","startedAt":"2025-06-04T20:55:06Z","lastAt":"2025-06-05T23:13:07Z","messageCount":16,"participants":["Jeff King","Collin Funk","Junio C Hamano","Ramsay Jones","Patrick Steinhardt","Daniel Stenberg","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"519693","messageId":"20250604205505.GA1510724@coredump.intra.peff.net","threadId":"63585","inReplyTo":null,"subject":"[PATCH 0/3] silencing warnings with curl 8.14","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-04T20:55:05Z","receivedAt":"2025-06-04T20:55:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The new version of curl (which hit Debian unstable a few days ago)\ncauses a bunch of compiler warnings because we are passing regular ints\nto curl_easy_setopt() instead of longs. Passing longs has always been\nwhat you're supposed to do, but the new version is better about\ngenerating warnings with gcc (I think the type-check has been there for\na long time, but I gather it was broken and recently fixed).\n\nI split this into three patches since the solutions vary slightly (well,\nthe last two are the same, but my pontificating on the solution varies).\n\n  [1/3]: curl: fix integer constant typechecks with curl_easy_setopt()\n  [2/3]: curl: fix integer variable typechecks with curl_easy_setopt()\n  [3/3]: curl: fix symbolic constant typechecks with curl_easy_setopt()\n\n http-push.c   |  2 +-\n http.c        | 28 ++++++++++++++--------------\n imap-send.c   |  6 +++---\n remote-curl.c |  6 +++---\n 4 files changed, 21 insertions(+), 21 deletions(-)\n\n-Peff\n"},{"id":"519694","messageId":"20250604205513.GA1510819@coredump.intra.peff.net","threadId":"63585","inReplyTo":"20250604205505.GA1510724@coredump.intra.peff.net","subject":"[PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-04T20:55:13Z","receivedAt":"2025-06-04T20:55:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The curl documentation specifies that curl_easy_setopt() takes either:\n\n  ...a long, a function pointer, an object pointer or a curl_off_t,\n  depending on what the specific option expects.\n\nBut when we pass an integer constant like \"0\", it will by default be a\nregular non-long int. This has always been wrong, but seemed to work in\npractice (I didn't dig into curl's implementation to see whether this\nmight actually be triggering undefined behavior, but it seems likely and\nregardless we should do what the docs say).\n\nThis is especially important since curl has a type-checking macro that\ncauses building against curl 8.14 to produce many warnings. The specific\ncommit is due to their 79b4e56b3 (typecheck-gcc.h: fix the typechecks,\n2025-04-22). Curiously, it does only seem to trigger when compiled with\n-O2 for me.\n\nWe can fix it by just marking the constants with a long \"L\".\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n http-push.c   |  2 +-\n http.c        | 14 +++++++-------\n remote-curl.c |  6 +++---\n 3 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex f9e67cabd4..591e46ab26 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -195,7 +195,7 @@ static char *xml_entities(const char *s)\n static void curl_setup_http_get(CURL *curl, const char *url,\n \t\tconst char *custom_req)\n {\n-\tcurl_easy_setopt(curl, CURLOPT_HTTPGET, 1);\n+\tcurl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);\n \tcurl_easy_setopt(curl, CURLOPT_URL, url);\n \tcurl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);\n \tcurl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, fwrite_null);\ndiff --git a/http.c b/http.c\nindex 3c029cf894..cce2ea7287 100644\n--- a/http.c\n+++ b/http.c\n@@ -1019,13 +1019,13 @@ static CURL *get_curl_handle(void)\n \t\tdie(\"curl_easy_init failed\");\n \n \tif (!curl_ssl_verify) {\n-\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0);\n-\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0);\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 0L);\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 0L);\n \t} else {\n \t\t/* Verify authenticity of the peer's certificate */\n-\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 1);\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYPEER, 1L);\n \t\t/* The name in the cert must match whom we tried to connect */\n-\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2);\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_VERIFYHOST, 2L);\n \t}\n \n     if (curl_http_version) {\n@@ -1117,7 +1117,7 @@ static CURL *get_curl_handle(void)\n \t\t\t\t curl_low_speed_time);\n \t}\n \n-\tcurl_easy_setopt(result, CURLOPT_MAXREDIRS, 20);\n+\tcurl_easy_setopt(result, CURLOPT_MAXREDIRS, 20L);\n \tcurl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n \n #ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR\n@@ -1151,7 +1151,7 @@ static CURL *get_curl_handle(void)\n \t\tuser_agent ? user_agent : git_user_agent());\n \n \tif (curl_ftp_no_epsv)\n-\t\tcurl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);\n+\t\tcurl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0L);\n \n \tif (curl_ssl_try)\n \t\tcurl_easy_setopt(result, CURLOPT_USE_SSL, CURLUSESSL_TRY);\n@@ -1254,7 +1254,7 @@ static CURL *get_curl_handle(void)\n \t}\n \tinit_curl_proxy_auth(result);\n \n-\tcurl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1);\n+\tcurl_easy_setopt(result, CURLOPT_TCP_KEEPALIVE, 1L);\n \n \tif (curl_tcp_keepidle > -1)\n \t\tcurl_easy_setopt(result, CURLOPT_TCP_KEEPIDLE,\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 590b228f67..6183772191 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -877,12 +877,12 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)\n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n \theaders = curl_slist_append(headers, rpc->hdr_accept);\n \n-\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1L);\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, \"0000\");\n-\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4L);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &buf);\n-- \n2.50.0.rc1.276.g7db1193dde\n\n"},{"id":"519695","messageId":"20250604205552.GB1510819@coredump.intra.peff.net","threadId":"63585","inReplyTo":"20250604205505.GA1510724@coredump.intra.peff.net","subject":"[PATCH 2/3] curl: fix integer variable typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-04T20:55:52Z","receivedAt":"2025-06-04T20:55:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"As discussed in the previous commit, we should be passing long integers,\nnot regular ones, to curl_easy_setopt(), and compiling against curl 8.14\nloudly complains if we don't.\n\nThat patch fixed integer constants by adding an \"L\". This one deals with\nactual variables.\n\nArguably these variables could just be declared as \"long\" in the first\nplace. But it's actually kind of awkward due to other code which uses\nthem:\n\n  - port is conceptually a short, and we even call htons() on it (though\n    weirdly it is defined as a regular int).\n\n  - ssl_verify is conceptually a bool, and we assign to it from\n    git_config_bool().\n\nSo I think we could probably switch these out for longs without hurting\nanything, but it just feels a bit weird. Doubly so because if you don't\nset USE_CURL_FOR_IMAP_SEND set, then the current types are fine!\n\nSo let's just cast these to longs in the curl calls, which makes what's\ngoing on obvious. There aren't that many spots to modify (and as you can\nsee from the context, we already have some similar casts).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n imap-send.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 27dc033c7f..2e812f5a6e 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1420,7 +1420,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n \n \tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n \tstrbuf_release(&path);\n-\tcurl_easy_setopt(curl, CURLOPT_PORT, srvc->port);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, (long)srvc->port);\n \n \tif (srvc->auth_method) {\n \t\tstruct strbuf auth = STRBUF_INIT;\n@@ -1433,8 +1433,8 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n \tif (!srvc->use_ssl)\n \t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_TRY);\n \n-\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, srvc->ssl_verify);\n-\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, srvc->ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, (long)srvc->ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, (long)srvc->ssl_verify);\n \n \tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n \n-- \n2.50.0.rc1.276.g7db1193dde\n\n"},{"id":"519696","messageId":"20250604205622.GC1510819@coredump.intra.peff.net","threadId":"63585","inReplyTo":"20250604205505.GA1510724@coredump.intra.peff.net","subject":"[PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-04T20:56:22Z","receivedAt":"2025-06-04T20:56:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"As with the previous two commits, we should be passing long integers,\nnot regular ones, to curl_easy_setopt(), and compiling against curl 8.14\nloudly complains if we don't.\n\nThis patch catches the remaining cases, which are ones where we pass\ncurl's own symbolic constants. We'll cast them to long manually in each\ncall.\n\nIt seems kind of weird to me that curl doesn't define these constants as\nlongs, since the point of them is to pass to curl_easy_setopt(). But in\nthe curl documentation and examples, they clearly show casting them as\npart of the setopt calls. It may be that there is some reason not to\npush the type into the macro, like backwards compatibility. I didn't\ndig, as it doesn't really matter: we have to follow what existing curl\nversions ask for anyway.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n http.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex cce2ea7287..ecbc47ea4b 100644\n--- a/http.c\n+++ b/http.c\n@@ -1057,7 +1057,7 @@ static CURL *get_curl_handle(void)\n \n \tif (http_ssl_backend && !strcmp(\"schannel\", http_ssl_backend) &&\n \t    !http_schannel_check_revoke) {\n-\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, CURLSSLOPT_NO_REVOKE);\n+\t\tcurl_easy_setopt(result, CURLOPT_SSL_OPTIONS, (long)CURLSSLOPT_NO_REVOKE);\n \t}\n \n \tif (http_proactive_auth != PROACTIVE_AUTH_NONE)\n@@ -1118,7 +1118,7 @@ static CURL *get_curl_handle(void)\n \t}\n \n \tcurl_easy_setopt(result, CURLOPT_MAXREDIRS, 20L);\n-\tcurl_easy_setopt(result, CURLOPT_POSTREDIR, CURL_REDIR_POST_ALL);\n+\tcurl_easy_setopt(result, CURLOPT_POSTREDIR, (long)CURL_REDIR_POST_ALL);\n \n #ifdef GIT_CURL_HAVE_CURLOPT_PROTOCOLS_STR\n \t{\n@@ -1193,18 +1193,18 @@ static CURL *get_curl_handle(void)\n \n \t\tif (starts_with(curl_http_proxy, \"socks5h\"))\n \t\t\tcurl_easy_setopt(result,\n-\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS5_HOSTNAME);\n+\t\t\t\tCURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS5_HOSTNAME);\n \t\telse if (starts_with(curl_http_proxy, \"socks5\"))\n \t\t\tcurl_easy_setopt(result,\n-\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS5);\n+\t\t\t\tCURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS5);\n \t\telse if (starts_with(curl_http_proxy, \"socks4a\"))\n \t\t\tcurl_easy_setopt(result,\n-\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS4A);\n+\t\t\t\tCURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS4A);\n \t\telse if (starts_with(curl_http_proxy, \"socks\"))\n \t\t\tcurl_easy_setopt(result,\n-\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);\n+\t\t\t\tCURLOPT_PROXYTYPE, (long)CURLPROXY_SOCKS4);\n \t\telse if (starts_with(curl_http_proxy, \"https\")) {\n-\t\t\tcurl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_HTTPS);\n+\t\t\tcurl_easy_setopt(result, CURLOPT_PROXYTYPE, (long)CURLPROXY_HTTPS);\n \n \t\t\tif (http_proxy_ssl_cert)\n \t\t\t\tcurl_easy_setopt(result, CURLOPT_PROXY_SSLCERT, http_proxy_ssl_cert);\n-- \n2.50.0.rc1.276.g7db1193dde\n"},{"id":"519699","messageId":"m1ecvzb3qu.fsf@gmail.com","threadId":"63585","inReplyTo":"20250604205505.GA1510724@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] silencing warnings with curl 8.14","fromName":"Collin Funk","fromEmail":"collin.funk1@gmail.com","sentAt":"2025-06-04T21:23:21Z","receivedAt":"2025-06-04T21:23:22Z","isPatch":true,"sender":{"key":"collin.funk1@gmail.com","avatar":"https://avatars.githubusercontent.com/u/65689063?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The new version of curl (which hit Debian unstable a few days ago)\n> causes a bunch of compiler warnings because we are passing regular ints\n> to curl_easy_setopt() instead of longs. Passing longs has always been\n> what you're supposed to do, but the new version is better about\n> generating warnings with gcc (I think the type-check has been there for\n> a long time, but I gather it was broken and recently fixed).\n>\n> I split this into three patches since the solutions vary slightly (well,\n> the last two are the same, but my pontificating on the solution varies).\n>\n>   [1/3]: curl: fix integer constant typechecks with curl_easy_setopt()\n>   [2/3]: curl: fix integer variable typechecks with curl_easy_setopt()\n>   [3/3]: curl: fix symbolic constant typechecks with curl_easy_setopt()\n\nI saw some GitHub CI's fail yesterday due to this as well [1], but can't\nseem to find the exact error logs at the moment...\n\nAnyways, I came to the same conclusion as your patches. So:\n\nReviewed-by: Collin Funk <collin.funk1@gmail.com>\n\nThanks,\nCollin\n\n[1] https://github.com/git/git\n"},{"id":"519700","messageId":"xmqq8qm7b3nu.fsf@gitster.g","threadId":"63585","inReplyTo":"20250604205622.GC1510819@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-04T21:25:09Z","receivedAt":"2025-06-04T21:25:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It seems kind of weird to me that curl doesn't define these constants as\n> longs, since the point of them is to pass to curl_easy_setopt(). But in\n> the curl documentation and examples, they clearly show casting them as\n> part of the setopt calls. It may be that there is some reason not to\n> push the type into the macro, like backwards compatibility. I didn't\n> dig, as it doesn't really matter: we have to follow what existing curl\n> versions ask for anyway.\n\nWell reasoned, and I grew 100% with the above reasoning.\n\nThank you very much for putting these together.  Will queue.\n\n"},{"id":"519702","messageId":"769e85c6-c7f7-4732-881a-5765c6ca2410@ramsayjones.plus.com","threadId":"63585","inReplyTo":"20250604205505.GA1510724@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] silencing warnings with curl 8.14","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2025-06-04T22:03:10Z","receivedAt":"2025-06-04T22:06:21Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 04/06/2025 21:55, Jeff King wrote:\n> The new version of curl (which hit Debian unstable a few days ago)\n> causes a bunch of compiler warnings because we are passing regular ints\n> to curl_easy_setopt() instead of longs. Passing longs has always been\n> what you're supposed to do, but the new version is better about\n> generating warnings with gcc (I think the type-check has been there for\n> a long time, but I gather it was broken and recently fixed).\n\nYep, I updated cygwin the other night and curl had been updated, so I\nsaw exactly the same ...\n\n> \n> I split this into three patches since the solutions vary slightly (well,\n> the last two are the same, but my pontificating on the solution varies).\n> \n>   [1/3]: curl: fix integer constant typechecks with curl_easy_setopt()\n>   [2/3]: curl: fix integer variable typechecks with curl_easy_setopt()\n>   [3/3]: curl: fix symbolic constant typechecks with curl_easy_setopt()\n\n.. and came up with the same (single) patch, which I was going to split\ninto three! :)\n\nHowever, I also looked into what a patch to curl would look like to change\nthe constants in patch #3 to long constants. Until I read your commit\nmessage, I didn't think there would be much of a problem ... :)\n\nThanks.\n\nATB,\nRamsay Jones\n\n\n"},{"id":"519707","messageId":"aEEpYQsE36skWxk5@pks.im","threadId":"63585","inReplyTo":"20250604205505.GA1510724@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] silencing warnings with curl 8.14","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-06-05T05:21:37Z","receivedAt":"2025-06-05T05:21:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jun 04, 2025 at 04:55:05PM -0400, Jeff King wrote:\n> The new version of curl (which hit Debian unstable a few days ago)\n> causes a bunch of compiler warnings because we are passing regular ints\n> to curl_easy_setopt() instead of longs. Passing longs has always been\n> what you're supposed to do, but the new version is better about\n> generating warnings with gcc (I think the type-check has been there for\n> a long time, but I gather it was broken and recently fixed).\n> \n> I split this into three patches since the solutions vary slightly (well,\n> the last two are the same, but my pontificating on the solution varies).\n\nAll of these look good to me, thanks!\n\nPatrick\n"},{"id":"519709","messageId":"r1197994-o3so-6453-q16n-6n3on33n4nrp@unkk.fr","threadId":"63585","inReplyTo":"20250604205622.GC1510819@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2025-06-05T06:13:09Z","receivedAt":"2025-06-05T06:20:15Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Wed, 4 Jun 2025, Jeff King wrote:\n\n> It seems kind of weird to me that curl doesn't define these constants as\n> longs, since the point of them is to pass to curl_easy_setopt().\n\nAgreed. Mostly just because of my lack of imagination when I added them a long \ntime ago.\n\nWe have over recent times updated several public option related defines to \nbetter help applications to get int vs long right, but I have clearly missed \nto do that for this particular set.\n\nI intend to fix this omission, but since you want to support building with \nlots of old curl versions as well, this correction probably won't help you for \nanother decade or so... :-)\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"519731","messageId":"20250605072504.GB2066712@coredump.intra.peff.net","threadId":"63585","inReplyTo":"r1197994-o3so-6453-q16n-6n3on33n4nrp@unkk.fr","subject":"Re: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-05T07:25:04Z","receivedAt":"2025-06-05T07:25:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2025 at 08:13:09AM +0200, Daniel Stenberg wrote:\n\n> On Wed, 4 Jun 2025, Jeff King wrote:\n> \n> > It seems kind of weird to me that curl doesn't define these constants as\n> > longs, since the point of them is to pass to curl_easy_setopt().\n> \n> Agreed. Mostly just because of my lack of imagination when I added them a\n> long time ago.\n\nOh, OK. :)\n\n> We have over recent times updated several public option related defines to\n> better help applications to get int vs long right, but I have clearly missed\n> to do that for this particular set.\n> \n> I intend to fix this omission, but since you want to support building with\n> lots of old curl versions as well, this correction probably won't help you\n> for another decade or so... :-)\n\nSounds like a good plan. But yeah, we'll want to continue with the casts\nhere for a while.\n\n-Peff\n"},{"id":"519762","messageId":"9bd5f0f3-d0c5-067b-ffa6-12a2c0353580@gmx.de","threadId":"63585","inReplyTo":"20250604205513.GA1510819@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-06-05T10:57:35Z","receivedAt":"2025-06-05T10:57:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jeff,\n\nOn Wed, 4 Jun 2025, Jeff King wrote:\n\n> The curl documentation specifies that curl_easy_setopt() takes either:\n> \n>   ...a long, a function pointer, an object pointer or a curl_off_t,\n>   depending on what the specific option expects.\n> \n> But when we pass an integer constant like \"0\", it will by default be a\n> regular non-long int. This has always been wrong, but seemed to work in\n> practice (I didn't dig into curl's implementation to see whether this\n> might actually be triggering undefined behavior, but it seems likely and\n> regardless we should do what the docs say).\n\nThe `curl_easy_setopt()` function takes the parameter as a vararg to allow\nfor multiple types. That means that 32-bit systems wouldn't see a\ndifference (where commonly `int` and `long` are both 4 bytes wide).\nWindows (and other LLP64 systems, if they exist) would be fine, too. But\non LP64 systems like Linux/macOS, it would make a difference. It might\nwork \"by mistake\" on little-endian systems if by happenstance the\nremaining 4 bytes are zero.\n\n> This is especially important since curl has a type-checking macro that\n> causes building against curl 8.14 to produce many warnings. The specific\n> commit is due to their 79b4e56b3 (typecheck-gcc.h: fix the typechecks,\n> 2025-04-22). Curiously, it does only seem to trigger when compiled with\n> -O2 for me.\n> \n> We can fix it by just marking the constants with a long \"L\".\n\nI just offered an alternative in\nhttps://lore.kernel.org/git/pull.1931.git.1749112304079.gitgitgadget@gmail.com/,\nbeing unaware of your efforts.\n\nMine was driven by the failing `osx-gcc` job, and curiously after\n(changing all the `l`s to `L`s and) rebasing to your series, I still have\nthis:\n\n-- snip --\nSubject: [PATCH] curl: pass `long` values where expected\n\nAs of Homebrew's update to cURL v8.14.0, there are new compile errors to\nbe observed in the `osx-gcc` job of Git's CI builds:\n\n  In file included from http.h:8,\n                   from imap-send.c:36:\n  In function 'setup_curl',\n      inlined from 'curl_append_msgs_to_imap' at imap-send.c:1460:9,\n      inlined from 'cmd_main' at imap-send.c:1581:9:\n  /usr/local/Cellar/curl/8.14.0/include/curl/typecheck-gcc.h:50:15: error: call to '_curl_easy_setopt_err_long' declared with attribute warning: curl_easy_setopt expects a long argument [-Werror=attribute-warning]\n     50 |               _curl_easy_setopt_err_long();                             \\\n        |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~\n  /usr/local/Cellar/curl/8.14.0/include/curl/curl.h:54:7: note: in definition of macro 'CURL_IGNORE_DEPRECATION'\n     54 |       statements \\\n        |       ^~~~~~~~~~\n  imap-send.c:1423:9: note: in expansion of macro 'curl_easy_setopt'\n   1423 |         curl_easy_setopt(curl, CURLOPT_PORT, srvc->port);\n        |         ^~~~~~~~~~~~~~~~\n  [... many more instances of nearly identical warnings...]\n\nSee for example this CI workflow run:\nhttps://github.com/git/git/actions/runs/15454602308/job/43504278284#step:4:307\n\nThe most likely explanation is the entry \"typecheck-gcc.h: fix the\ntypechecks\" in cURL's release notes (https://curl.se/ch/8.14.0.html).\n\nLet's explicitly convert all `int` parameters in `curl_easy_setopt()`\ncalls to `long` parameters.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n http-push.c   |  6 +++---\n http.c        | 22 +++++++++++-----------\n remote-curl.c |  6 +++---\n 3 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 591e46ab260d..f5a92529a840 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -205,7 +205,7 @@ static void curl_setup_http(CURL *curl, const char *url,\n \t\tconst char *custom_req, struct buffer *buffer,\n \t\tcurl_write_callback write_fn)\n {\n-\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1);\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n \tcurl_easy_setopt(curl, CURLOPT_URL, url);\n \tcurl_easy_setopt(curl, CURLOPT_INFILE, buffer);\n \tcurl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);\n@@ -213,9 +213,9 @@ static void curl_setup_http(CURL *curl, const char *url,\n \tcurl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);\n \tcurl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);\n \tcurl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);\n-\tcurl_easy_setopt(curl, CURLOPT_NOBODY, 0);\n+\tcurl_easy_setopt(curl, CURLOPT_NOBODY, 0L);\n \tcurl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);\n-\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1);\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n }\n \n static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)\ndiff --git a/http.c b/http.c\nindex ecbc47ea4b3f..d88e79fbde9c 100644\n--- a/http.c\n+++ b/http.c\n@@ -1540,9 +1540,9 @@ struct active_request_slot *get_active_slot(void)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, -1L);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1L);\n \tcurl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);\n \n \t/*\n@@ -1551,9 +1551,9 @@ struct active_request_slot *get_active_slot(void)\n \t * HTTP_FOLLOW_* cases themselves.\n \t */\n \tif (http_follow_config == HTTP_FOLLOW_ALWAYS)\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);\n \telse\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0L);\n \n \tcurl_easy_setopt(slot->curl, CURLOPT_IPRESOLVE, git_curl_ipresolve);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);\n@@ -2120,12 +2120,12 @@ static int http_request(const char *url,\n \tint ret;\n \n \tslot = get_active_slot();\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);\n \n \tif (!result) {\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1L);\n \t} else {\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, result);\n \n \t\tif (target == HTTP_REQUEST_FILE) {\n@@ -2151,7 +2151,7 @@ static int http_request(const char *url,\n \t\tstrbuf_addstr(&buf, \" no-cache\");\n \tif (options && options->initial_request &&\n \t    http_follow_config == HTTP_FOLLOW_INITIAL)\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);\n \n \theaders = curl_slist_append(headers, buf.buf);\n \n@@ -2170,7 +2170,7 @@ static int http_request(const char *url,\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);\n \n \tret = run_one_slot(slot, &results);\n \n@@ -2750,7 +2750,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \tfreq->headers = object_request_headers();\n \n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq);\n-\tcurl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);\n+\tcurl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0L);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 6183772191f2..b8bc3a80cf41 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -970,8 +970,8 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece\n \n \tslot = get_active_slot();\n \n-\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1L);\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n \n@@ -1058,7 +1058,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece\n \trpc_in_data.check_pktline = stateless_connect;\n \tmemset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state));\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &rpc_in_data);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);\n \n \n \trpc->any_written = 0;\n-- \n\nI wonder why you did not need those?\n\nIn any case, would you kindly adopt these changes into your patch series?\n\nThanks,\nJohannes\n"},{"id":"519789","messageId":"xmqqh60u9nuo.fsf@gitster.g","threadId":"63585","inReplyTo":"9bd5f0f3-d0c5-067b-ffa6-12a2c0353580@gmx.de","subject":"Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-05T16:04:15Z","receivedAt":"2025-06-05T16:04:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Mine was driven by the failing `osx-gcc` job, and curiously after\n> (changing all the `l`s to `L`s and) rebasing to your series, I still have\n> this:\n\nThanks.  Will queue but we probably want to reword the proposed log\nmessage to also refer to Peff's changes (i.e. \"That series covered\nsome, but here are a bit more\")?\n\n>\n> -- snip --\n\nI have been meaning to raise this since this is probably third or\nfourth time in the recent past, but every time I forgot to do so\nX-<.  This is not something \"am -c\" recognises as a scissors line.\n\nI'll queue this on top of the other three-patch series.\n\nThanks.\n\n--- >8 ---\nFrom: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nDate: Thu, 5 Jun 2025 12:57:35 +0200\nSubject: [PATCH] curl: pass `long` values where expected\n\nA set of patches posted by Jeff King earlier covered some fallouts\ncoming from new typecheck warnings cURL 8.14.0.  Here are to fix\nsome more instances of the same new compile errors observed in the\n`osx-gcc` job of Git's CI builds.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n http-push.c   |  6 +++---\n http.c        | 22 +++++++++++-----------\n remote-curl.c |  6 +++---\n 3 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 591e46ab26..f5a92529a8 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -205,7 +205,7 @@ static void curl_setup_http(CURL *curl, const char *url,\n \t\tconst char *custom_req, struct buffer *buffer,\n \t\tcurl_write_callback write_fn)\n {\n-\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1);\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n \tcurl_easy_setopt(curl, CURLOPT_URL, url);\n \tcurl_easy_setopt(curl, CURLOPT_INFILE, buffer);\n \tcurl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);\n@@ -213,9 +213,9 @@ static void curl_setup_http(CURL *curl, const char *url,\n \tcurl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);\n \tcurl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);\n \tcurl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);\n-\tcurl_easy_setopt(curl, CURLOPT_NOBODY, 0);\n+\tcurl_easy_setopt(curl, CURLOPT_NOBODY, 0L);\n \tcurl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);\n-\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1);\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n }\n \n static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)\ndiff --git a/http.c b/http.c\nindex ecbc47ea4b..d88e79fbde 100644\n--- a/http.c\n+++ b/http.c\n@@ -1540,9 +1540,9 @@ struct active_request_slot *get_active_slot(void)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, -1L);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1L);\n \tcurl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);\n \n \t/*\n@@ -1551,9 +1551,9 @@ struct active_request_slot *get_active_slot(void)\n \t * HTTP_FOLLOW_* cases themselves.\n \t */\n \tif (http_follow_config == HTTP_FOLLOW_ALWAYS)\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);\n \telse\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0L);\n \n \tcurl_easy_setopt(slot->curl, CURLOPT_IPRESOLVE, git_curl_ipresolve);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);\n@@ -2120,12 +2120,12 @@ static int http_request(const char *url,\n \tint ret;\n \n \tslot = get_active_slot();\n-\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);\n \n \tif (!result) {\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1L);\n \t} else {\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, result);\n \n \t\tif (target == HTTP_REQUEST_FILE) {\n@@ -2151,7 +2151,7 @@ static int http_request(const char *url,\n \t\tstrbuf_addstr(&buf, \" no-cache\");\n \tif (options && options->initial_request &&\n \t    http_follow_config == HTTP_FOLLOW_INITIAL)\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);\n \n \theaders = curl_slist_append(headers, buf.buf);\n \n@@ -2170,7 +2170,7 @@ static int http_request(const char *url,\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);\n \n \tret = run_one_slot(slot, &results);\n \n@@ -2750,7 +2750,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \tfreq->headers = object_request_headers();\n \n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq);\n-\tcurl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);\n+\tcurl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0L);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 6183772191..b8bc3a80cf 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -970,8 +970,8 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece\n \n \tslot = get_active_slot();\n \n-\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1L);\n \tcurl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);\n \tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n \n@@ -1058,7 +1058,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece\n \trpc_in_data.check_pktline = stateless_connect;\n \tmemset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state));\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &rpc_in_data);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);\n \n \n \trpc->any_written = 0;\n-- \n2.50.0-rc1-198-g2c07f1279d\n\n"},{"id":"519809","messageId":"20250605224747.GA3005733@coredump.intra.peff.net","threadId":"63585","inReplyTo":"9bd5f0f3-d0c5-067b-ffa6-12a2c0353580@gmx.de","subject":"Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-05T22:47:47Z","receivedAt":"2025-06-05T22:47:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2025 at 12:57:35PM +0200, Johannes Schindelin wrote:\n\n> > But when we pass an integer constant like \"0\", it will by default be a\n> > regular non-long int. This has always been wrong, but seemed to work in\n> > practice (I didn't dig into curl's implementation to see whether this\n> > might actually be triggering undefined behavior, but it seems likely and\n> > regardless we should do what the docs say).\n> \n> The `curl_easy_setopt()` function takes the parameter as a vararg to allow\n> for multiple types. That means that 32-bit systems wouldn't see a\n> difference (where commonly `int` and `long` are both 4 bytes wide).\n> Windows (and other LLP64 systems, if they exist) would be fine, too. But\n> on LP64 systems like Linux/macOS, it would make a difference. It might\n> work \"by mistake\" on little-endian systems if by happenstance the\n> remaining 4 bytes are zero.\n\nThat was my intuition as well, but then I'd think it would be failing\nreliably on big-endian LP64 systems. But maybe nobody is using such a\nsystem? I _thought_ building on Android might get us there (something I\ndo myself sometimes), but at least my ARM64 device is little-endian\n(apparently it's bi-endian but defaults to little).\n\nSo maybe it's a problem waiting to happen and we just haven't seen it.\n\nAt any rate, that is all just curiosity and I don't think changes what\nthe patch should do.\n\n> Mine was driven by the failing `osx-gcc` job, and curiously after\n> (changing all the `l`s to `L`s and) rebasing to your series, I still have\n> this:\n\nInteresting. As you might guess, mine was driven by fixing the compiler\nwarnings I was seeing on Linux, and I didn't do a full audit of all\ncalls (since doing so requires cross-referencing the expected type for\nevery CURLOPT specifier).\n\nI wonder why these extra cases are caught on macOS but not Linux?\n\nIt is probably another mystery not really worth resolving, as clearly\nthe right thing here is to fix them, as your patch does.\n\n-Peff\n"},{"id":"519810","messageId":"20250605224910.GB3005733@coredump.intra.peff.net","threadId":"63585","inReplyTo":"xmqqh60u9nuo.fsf@gitster.g","subject":"Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-05T22:49:10Z","receivedAt":"2025-06-05T22:49:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2025 at 09:04:15AM -0700, Junio C Hamano wrote:\n\n> --- >8 ---\n> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Date: Thu, 5 Jun 2025 12:57:35 +0200\n> Subject: [PATCH] curl: pass `long` values where expected\n> \n> A set of patches posted by Jeff King earlier covered some fallouts\n> coming from new typecheck warnings cURL 8.14.0.  Here are to fix\n> some more instances of the same new compile errors observed in the\n> `osx-gcc` job of Git's CI builds.\n> \n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nThanks, this patch looks good, and I think applying on top is a bit less\nwork. I don't mind integrating them appropriately and re-rolling if we\nprefer a slightly cleaner history, though. (I don't think there's much\nvalue in recording which hit macOS and which did not).\n\n-Peff\n"},{"id":"519812","messageId":"20250605225144.GD3005733@coredump.intra.peff.net","threadId":"63585","inReplyTo":"20250605224910.GB3005733@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-05T22:51:44Z","receivedAt":"2025-06-05T22:51:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 05, 2025 at 06:49:10PM -0400, Jeff King wrote:\n\n> On Thu, Jun 05, 2025 at 09:04:15AM -0700, Junio C Hamano wrote:\n> \n> > --- >8 ---\n> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> > Date: Thu, 5 Jun 2025 12:57:35 +0200\n> > Subject: [PATCH] curl: pass `long` values where expected\n> > \n> > A set of patches posted by Jeff King earlier covered some fallouts\n> > coming from new typecheck warnings cURL 8.14.0.  Here are to fix\n> > some more instances of the same new compile errors observed in the\n> > `osx-gcc` job of Git's CI builds.\n> > \n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> \n> Thanks, this patch looks good, and I think applying on top is a bit less\n> work. I don't mind integrating them appropriately and re-rolling if we\n> prefer a slightly cleaner history, though. (I don't think there's much\n> value in recording which hit macOS and which did not).\n\nAh, nevermind, my patches are already in next, so building on top is\ndefinitely best.\n\n-Peff\n"},{"id":"519813","messageId":"xmqq34cd7pfj.fsf@gitster.g","threadId":"63585","inReplyTo":"20250605225144.GD3005733@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-05T23:13:04Z","receivedAt":"2025-06-05T23:13:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Jun 05, 2025 at 06:49:10PM -0400, Jeff King wrote:\n>\n>> On Thu, Jun 05, 2025 at 09:04:15AM -0700, Junio C Hamano wrote:\n>> \n>> > --- >8 ---\n>> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n>> > Date: Thu, 5 Jun 2025 12:57:35 +0200\n>> > Subject: [PATCH] curl: pass `long` values where expected\n>> > \n>> > A set of patches posted by Jeff King earlier covered some fallouts\n>> > coming from new typecheck warnings cURL 8.14.0.  Here are to fix\n>> > some more instances of the same new compile errors observed in the\n>> > `osx-gcc` job of Git's CI builds.\n>> > \n>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n>> > Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>> \n>> Thanks, this patch looks good, and I think applying on top is a bit less\n>> work. I don't mind integrating them appropriately and re-rolling if we\n>> prefer a slightly cleaner history, though. (I don't think there's much\n>> value in recording which hit macOS and which did not).\n\nYeah, other than giving a quick access to the places that only broke\nmacOS for those who are curious enough and want to find out why ;-)\n\n> Ah, nevermind, my patches are already in next, so building on top is\n> definitely best.\n\nYeah, in any case, taking all four of your patches together, with\nDscho's t5410 \"does tee hang?\" fix, finally lets the tip of 'seen'\npass without forcing retests.\n\n"}]}