git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] silencing warnings with curl 8.14
@ 2025-06-04 20:55 Jeff King
  2025-06-04 20:55 ` [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt() Jeff King
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Jeff King @ 2025-06-04 20:55 UTC (permalink / raw)
  To: git

The new version of curl (which hit Debian unstable a few days ago)
causes a bunch of compiler warnings because we are passing regular ints
to curl_easy_setopt() instead of longs. Passing longs has always been
what you're supposed to do, but the new version is better about
generating warnings with gcc (I think the type-check has been there for
a long time, but I gather it was broken and recently fixed).

I split this into three patches since the solutions vary slightly (well,
the last two are the same, but my pontificating on the solution varies).

  [1/3]: curl: fix integer constant typechecks with curl_easy_setopt()
  [2/3]: curl: fix integer variable typechecks with curl_easy_setopt()
  [3/3]: curl: fix symbolic constant typechecks with curl_easy_setopt()

 http-push.c   |  2 +-
 http.c        | 28 ++++++++++++++--------------
 imap-send.c   |  6 +++---
 remote-curl.c |  6 +++---
 4 files changed, 21 insertions(+), 21 deletions(-)

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
@ 2025-06-04 20:55 ` Jeff King
  2025-06-05 10:57   ` Johannes Schindelin
  2025-06-04 20:55 ` [PATCH 2/3] curl: fix integer variable " Jeff King
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2025-06-04 20:55 UTC (permalink / raw)
  To: git

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 <peff@peff.net>
---
 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);
-- 
2.50.0.rc1.276.g7db1193dde


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] curl: fix integer variable typechecks with curl_easy_setopt()
  2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
  2025-06-04 20:55 ` [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt() Jeff King
@ 2025-06-04 20:55 ` Jeff King
  2025-06-04 20:56 ` [PATCH 3/3] curl: fix symbolic constant " Jeff King
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2025-06-04 20:55 UTC (permalink / raw)
  To: git

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 <peff@peff.net>
---
 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);
 
-- 
2.50.0.rc1.276.g7db1193dde


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()
  2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
  2025-06-04 20:55 ` [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt() Jeff King
  2025-06-04 20:55 ` [PATCH 2/3] curl: fix integer variable " Jeff King
@ 2025-06-04 20:56 ` Jeff King
  2025-06-04 21:25   ` Junio C Hamano
  2025-06-05  6:13   ` Daniel Stenberg
  2025-06-04 21:23 ` [PATCH 0/3] silencing warnings with curl 8.14 Collin Funk
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Jeff King @ 2025-06-04 20:56 UTC (permalink / raw)
  To: git

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 <peff@peff.net>
---
 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);
-- 
2.50.0.rc1.276.g7db1193dde

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] silencing warnings with curl 8.14
  2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
                   ` (2 preceding siblings ...)
  2025-06-04 20:56 ` [PATCH 3/3] curl: fix symbolic constant " Jeff King
@ 2025-06-04 21:23 ` Collin Funk
  2025-06-04 22:03 ` Ramsay Jones
  2025-06-05  5:21 ` Patrick Steinhardt
  5 siblings, 0 replies; 16+ messages in thread
From: Collin Funk @ 2025-06-04 21:23 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Jeff King <peff@peff.net> writes:

> The new version of curl (which hit Debian unstable a few days ago)
> causes a bunch of compiler warnings because we are passing regular ints
> to curl_easy_setopt() instead of longs. Passing longs has always been
> what you're supposed to do, but the new version is better about
> generating warnings with gcc (I think the type-check has been there for
> a long time, but I gather it was broken and recently fixed).
>
> I split this into three patches since the solutions vary slightly (well,
> the last two are the same, but my pontificating on the solution varies).
>
>   [1/3]: curl: fix integer constant typechecks with curl_easy_setopt()
>   [2/3]: curl: fix integer variable typechecks with curl_easy_setopt()
>   [3/3]: curl: fix symbolic constant typechecks with curl_easy_setopt()

I saw some GitHub CI's fail yesterday due to this as well [1], but can't
seem to find the exact error logs at the moment...

Anyways, I came to the same conclusion as your patches. So:

Reviewed-by: Collin Funk <collin.funk1@gmail.com>

Thanks,
Collin

[1] https://github.com/git/git

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()
  2025-06-04 20:56 ` [PATCH 3/3] curl: fix symbolic constant " Jeff King
@ 2025-06-04 21:25   ` Junio C Hamano
  2025-06-05  6:13   ` Daniel Stenberg
  1 sibling, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2025-06-04 21:25 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Jeff King <peff@peff.net> writes:

> 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.

Well reasoned, and I grew 100% with the above reasoning.

Thank you very much for putting these together.  Will queue.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] silencing warnings with curl 8.14
  2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
                   ` (3 preceding siblings ...)
  2025-06-04 21:23 ` [PATCH 0/3] silencing warnings with curl 8.14 Collin Funk
@ 2025-06-04 22:03 ` Ramsay Jones
  2025-06-05  5:21 ` Patrick Steinhardt
  5 siblings, 0 replies; 16+ messages in thread
From: Ramsay Jones @ 2025-06-04 22:03 UTC (permalink / raw)
  To: Jeff King, git



On 04/06/2025 21:55, Jeff King wrote:
> The new version of curl (which hit Debian unstable a few days ago)
> causes a bunch of compiler warnings because we are passing regular ints
> to curl_easy_setopt() instead of longs. Passing longs has always been
> what you're supposed to do, but the new version is better about
> generating warnings with gcc (I think the type-check has been there for
> a long time, but I gather it was broken and recently fixed).

Yep, I updated cygwin the other night and curl had been updated, so I
saw exactly the same ...

> 
> I split this into three patches since the solutions vary slightly (well,
> the last two are the same, but my pontificating on the solution varies).
> 
>   [1/3]: curl: fix integer constant typechecks with curl_easy_setopt()
>   [2/3]: curl: fix integer variable typechecks with curl_easy_setopt()
>   [3/3]: curl: fix symbolic constant typechecks with curl_easy_setopt()

.. and came up with the same (single) patch, which I was going to split
into three! :)

However, I also looked into what a patch to curl would look like to change
the constants in patch #3 to long constants. Until I read your commit
message, I didn't think there would be much of a problem ... :)

Thanks.

ATB,
Ramsay Jones



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/3] silencing warnings with curl 8.14
  2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
                   ` (4 preceding siblings ...)
  2025-06-04 22:03 ` Ramsay Jones
@ 2025-06-05  5:21 ` Patrick Steinhardt
  5 siblings, 0 replies; 16+ messages in thread
From: Patrick Steinhardt @ 2025-06-05  5:21 UTC (permalink / raw)
  To: Jeff King; +Cc: git

On Wed, Jun 04, 2025 at 04:55:05PM -0400, Jeff King wrote:
> The new version of curl (which hit Debian unstable a few days ago)
> causes a bunch of compiler warnings because we are passing regular ints
> to curl_easy_setopt() instead of longs. Passing longs has always been
> what you're supposed to do, but the new version is better about
> generating warnings with gcc (I think the type-check has been there for
> a long time, but I gather it was broken and recently fixed).
> 
> I split this into three patches since the solutions vary slightly (well,
> the last two are the same, but my pontificating on the solution varies).

All of these look good to me, thanks!

Patrick

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()
  2025-06-04 20:56 ` [PATCH 3/3] curl: fix symbolic constant " Jeff King
  2025-06-04 21:25   ` Junio C Hamano
@ 2025-06-05  6:13   ` Daniel Stenberg
  2025-06-05  7:25     ` Jeff King
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Stenberg @ 2025-06-05  6:13 UTC (permalink / raw)
  To: Jeff King; +Cc: git

On Wed, 4 Jun 2025, Jeff King wrote:

> 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().

Agreed. Mostly just because of my lack of imagination when I added them a long 
time ago.

We have over recent times updated several public option related defines to 
better help applications to get int vs long right, but I have clearly missed 
to do that for this particular set.

I intend to fix this omission, but since you want to support building with 
lots of old curl versions as well, this correction probably won't help you for 
another decade or so... :-)

-- 

  / daniel.haxx.se

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] curl: fix symbolic constant typechecks with curl_easy_setopt()
  2025-06-05  6:13   ` Daniel Stenberg
@ 2025-06-05  7:25     ` Jeff King
  0 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2025-06-05  7:25 UTC (permalink / raw)
  To: Daniel Stenberg; +Cc: git

On Thu, Jun 05, 2025 at 08:13:09AM +0200, Daniel Stenberg wrote:

> On Wed, 4 Jun 2025, Jeff King wrote:
> 
> > 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().
> 
> Agreed. Mostly just because of my lack of imagination when I added them a
> long time ago.

Oh, OK. :)

> We have over recent times updated several public option related defines to
> better help applications to get int vs long right, but I have clearly missed
> to do that for this particular set.
> 
> I intend to fix this omission, but since you want to support building with
> lots of old curl versions as well, this correction probably won't help you
> for another decade or so... :-)

Sounds like a good plan. But yeah, we'll want to continue with the casts
here for a while.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-04 20:55 ` [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt() Jeff King
@ 2025-06-05 10:57   ` Johannes Schindelin
  2025-06-05 16:04     ` Junio C Hamano
  2025-06-05 22:47     ` Jeff King
  0 siblings, 2 replies; 16+ messages in thread
From: Johannes Schindelin @ 2025-06-05 10:57 UTC (permalink / raw)
  To: Jeff King; +Cc: git

Hi Jeff,

On Wed, 4 Jun 2025, Jeff King wrote:

> 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).

The `curl_easy_setopt()` function takes the parameter as a vararg to allow
for multiple types. That means that 32-bit systems wouldn't see a
difference (where commonly `int` and `long` are both 4 bytes wide).
Windows (and other LLP64 systems, if they exist) would be fine, too. But
on LP64 systems like Linux/macOS, it would make a difference. It might
work "by mistake" on little-endian systems if by happenstance the
remaining 4 bytes are zero.

> 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".

I just offered an alternative in
https://lore.kernel.org/git/pull.1931.git.1749112304079.gitgitgadget@gmail.com/,
being unaware of your efforts.

Mine was driven by the failing `osx-gcc` job, and curiously after
(changing all the `l`s to `L`s and) rebasing to your series, I still have
this:

-- snip --
Subject: [PATCH] curl: pass `long` values where expected

As of Homebrew's update to cURL v8.14.0, there are new compile errors to
be observed in the `osx-gcc` job of Git's CI builds:

  In file included from http.h:8,
                   from imap-send.c:36:
  In function 'setup_curl',
      inlined from 'curl_append_msgs_to_imap' at imap-send.c:1460:9,
      inlined from 'cmd_main' at imap-send.c:1581:9:
  /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]
     50 |               _curl_easy_setopt_err_long();                             \
        |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/local/Cellar/curl/8.14.0/include/curl/curl.h:54:7: note: in definition of macro 'CURL_IGNORE_DEPRECATION'
     54 |       statements \
        |       ^~~~~~~~~~
  imap-send.c:1423:9: note: in expansion of macro 'curl_easy_setopt'
   1423 |         curl_easy_setopt(curl, CURLOPT_PORT, srvc->port);
        |         ^~~~~~~~~~~~~~~~
  [... many more instances of nearly identical warnings...]

See for example this CI workflow run:
https://github.com/git/git/actions/runs/15454602308/job/43504278284#step:4:307

The most likely explanation is the entry "typecheck-gcc.h: fix the
typechecks" in cURL's release notes (https://curl.se/ch/8.14.0.html).

Let's explicitly convert all `int` parameters in `curl_easy_setopt()`
calls to `long` parameters.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 http-push.c   |  6 +++---
 http.c        | 22 +++++++++++-----------
 remote-curl.c |  6 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/http-push.c b/http-push.c
index 591e46ab260d..f5a92529a840 100644
--- a/http-push.c
+++ b/http-push.c
@@ -205,7 +205,7 @@ static void curl_setup_http(CURL *curl, const char *url,
 		const char *custom_req, struct buffer *buffer,
 		curl_write_callback write_fn)
 {
-	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
 	curl_easy_setopt(curl, CURLOPT_URL, url);
 	curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
 	curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
@@ -213,9 +213,9 @@ static void curl_setup_http(CURL *curl, const char *url,
 	curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
 	curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
 	curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
-	curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
+	curl_easy_setopt(curl, CURLOPT_NOBODY, 0L);
 	curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
-	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
 }
 
 static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)
diff --git a/http.c b/http.c
index ecbc47ea4b3f..d88e79fbde9c 100644
--- a/http.c
+++ b/http.c
@@ -1540,9 +1540,9 @@ struct active_request_slot *get_active_slot(void)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, NULL);
 	curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
 	curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, -1L);
-	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0L);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);
+	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1L);
 	curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);
 
 	/*
@@ -1551,9 +1551,9 @@ struct active_request_slot *get_active_slot(void)
 	 * HTTP_FOLLOW_* cases themselves.
 	 */
 	if (http_follow_config == HTTP_FOLLOW_ALWAYS)
-		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
+		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);
 	else
-		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0);
+		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0L);
 
 	curl_easy_setopt(slot->curl, CURLOPT_IPRESOLVE, git_curl_ipresolve);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
@@ -2120,12 +2120,12 @@ static int http_request(const char *url,
 	int ret;
 
 	slot = get_active_slot();
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);
 
 	if (!result) {
-		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1L);
 	} else {
-		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
 		curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, result);
 
 		if (target == HTTP_REQUEST_FILE) {
@@ -2151,7 +2151,7 @@ static int http_request(const char *url,
 		strbuf_addstr(&buf, " no-cache");
 	if (options && options->initial_request &&
 	    http_follow_config == HTTP_FOLLOW_INITIAL)
-		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
+		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);
 
 	headers = curl_slist_append(headers, buf.buf);
 
@@ -2170,7 +2170,7 @@ static int http_request(const char *url,
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);
 
 	ret = run_one_slot(slot, &results);
 
@@ -2750,7 +2750,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	freq->headers = object_request_headers();
 
 	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq);
-	curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
+	curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0L);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
diff --git a/remote-curl.c b/remote-curl.c
index 6183772191f2..b8bc3a80cf41 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -970,8 +970,8 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
 
 	slot = get_active_slot();
 
-	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, "");
 
@@ -1058,7 +1058,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
 	rpc_in_data.check_pktline = stateless_connect;
 	memset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state));
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &rpc_in_data);
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);
 
 
 	rpc->any_written = 0;
-- 

I wonder why you did not need those?

In any case, would you kindly adopt these changes into your patch series?

Thanks,
Johannes

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-05 10:57   ` Johannes Schindelin
@ 2025-06-05 16:04     ` Junio C Hamano
  2025-06-05 22:49       ` Jeff King
  2025-06-05 22:47     ` Jeff King
  1 sibling, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2025-06-05 16:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Mine was driven by the failing `osx-gcc` job, and curiously after
> (changing all the `l`s to `L`s and) rebasing to your series, I still have
> this:

Thanks.  Will queue but we probably want to reword the proposed log
message to also refer to Peff's changes (i.e. "That series covered
some, but here are a bit more")?

>
> -- snip --

I have been meaning to raise this since this is probably third or
fourth time in the recent past, but every time I forgot to do so
X-<.  This is not something "am -c" recognises as a scissors line.

I'll queue this on top of the other three-patch series.

Thanks.

--- >8 ---
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Thu, 5 Jun 2025 12:57:35 +0200
Subject: [PATCH] curl: pass `long` values where expected

A set of patches posted by Jeff King earlier covered some fallouts
coming from new typecheck warnings cURL 8.14.0.  Here are to fix
some more instances of the same new compile errors observed in the
`osx-gcc` job of Git's CI builds.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 http-push.c   |  6 +++---
 http.c        | 22 +++++++++++-----------
 remote-curl.c |  6 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/http-push.c b/http-push.c
index 591e46ab26..f5a92529a8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -205,7 +205,7 @@ static void curl_setup_http(CURL *curl, const char *url,
 		const char *custom_req, struct buffer *buffer,
 		curl_write_callback write_fn)
 {
-	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
 	curl_easy_setopt(curl, CURLOPT_URL, url);
 	curl_easy_setopt(curl, CURLOPT_INFILE, buffer);
 	curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
@@ -213,9 +213,9 @@ static void curl_setup_http(CURL *curl, const char *url,
 	curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
 	curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
 	curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_fn);
-	curl_easy_setopt(curl, CURLOPT_NOBODY, 0);
+	curl_easy_setopt(curl, CURLOPT_NOBODY, 0L);
 	curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, custom_req);
-	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
 }
 
 static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum dav_header_flag options)
diff --git a/http.c b/http.c
index ecbc47ea4b..d88e79fbde 100644
--- a/http.c
+++ b/http.c
@@ -1540,9 +1540,9 @@ struct active_request_slot *get_active_slot(void)
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, NULL);
 	curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, NULL);
 	curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, -1L);
-	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0);
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_UPLOAD, 0L);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);
+	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 1L);
 	curl_easy_setopt(slot->curl, CURLOPT_RANGE, NULL);
 
 	/*
@@ -1551,9 +1551,9 @@ struct active_request_slot *get_active_slot(void)
 	 * HTTP_FOLLOW_* cases themselves.
 	 */
 	if (http_follow_config == HTTP_FOLLOW_ALWAYS)
-		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
+		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);
 	else
-		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0);
+		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 0L);
 
 	curl_easy_setopt(slot->curl, CURLOPT_IPRESOLVE, git_curl_ipresolve);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPAUTH, http_auth_methods);
@@ -2120,12 +2120,12 @@ static int http_request(const char *url,
 	int ret;
 
 	slot = get_active_slot();
-	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);
+	curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1L);
 
 	if (!result) {
-		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1L);
 	} else {
-		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
+		curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0L);
 		curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, result);
 
 		if (target == HTTP_REQUEST_FILE) {
@@ -2151,7 +2151,7 @@ static int http_request(const char *url,
 		strbuf_addstr(&buf, " no-cache");
 	if (options && options->initial_request &&
 	    http_follow_config == HTTP_FOLLOW_INITIAL)
-		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1);
+		curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L);
 
 	headers = curl_slist_append(headers, buf.buf);
 
@@ -2170,7 +2170,7 @@ static int http_request(const char *url,
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
 	curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
 	curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);
 
 	ret = run_one_slot(slot, &results);
 
@@ -2750,7 +2750,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
 	freq->headers = object_request_headers();
 
 	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEDATA, freq);
-	curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0);
+	curl_easy_setopt(freq->slot->curl, CURLOPT_FAILONERROR, 0L);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr);
 	curl_easy_setopt(freq->slot->curl, CURLOPT_URL, freq->url);
diff --git a/remote-curl.c b/remote-curl.c
index 6183772191..b8bc3a80cf 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -970,8 +970,8 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
 
 	slot = get_active_slot();
 
-	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, "");
 
@@ -1058,7 +1058,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
 	rpc_in_data.check_pktline = stateless_connect;
 	memset(&rpc_in_data.pktline_state, 0, sizeof(rpc_in_data.pktline_state));
 	curl_easy_setopt(slot->curl, CURLOPT_WRITEDATA, &rpc_in_data);
-	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0);
+	curl_easy_setopt(slot->curl, CURLOPT_FAILONERROR, 0L);
 
 
 	rpc->any_written = 0;
-- 
2.50.0-rc1-198-g2c07f1279d


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-05 10:57   ` Johannes Schindelin
  2025-06-05 16:04     ` Junio C Hamano
@ 2025-06-05 22:47     ` Jeff King
  1 sibling, 0 replies; 16+ messages in thread
From: Jeff King @ 2025-06-05 22:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

On Thu, Jun 05, 2025 at 12:57:35PM +0200, Johannes Schindelin wrote:

> > 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).
> 
> The `curl_easy_setopt()` function takes the parameter as a vararg to allow
> for multiple types. That means that 32-bit systems wouldn't see a
> difference (where commonly `int` and `long` are both 4 bytes wide).
> Windows (and other LLP64 systems, if they exist) would be fine, too. But
> on LP64 systems like Linux/macOS, it would make a difference. It might
> work "by mistake" on little-endian systems if by happenstance the
> remaining 4 bytes are zero.

That was my intuition as well, but then I'd think it would be failing
reliably on big-endian LP64 systems. But maybe nobody is using such a
system? I _thought_ building on Android might get us there (something I
do myself sometimes), but at least my ARM64 device is little-endian
(apparently it's bi-endian but defaults to little).

So maybe it's a problem waiting to happen and we just haven't seen it.

At any rate, that is all just curiosity and I don't think changes what
the patch should do.

> Mine was driven by the failing `osx-gcc` job, and curiously after
> (changing all the `l`s to `L`s and) rebasing to your series, I still have
> this:

Interesting. As you might guess, mine was driven by fixing the compiler
warnings I was seeing on Linux, and I didn't do a full audit of all
calls (since doing so requires cross-referencing the expected type for
every CURLOPT specifier).

I wonder why these extra cases are caught on macOS but not Linux?

It is probably another mystery not really worth resolving, as clearly
the right thing here is to fix them, as your patch does.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-05 16:04     ` Junio C Hamano
@ 2025-06-05 22:49       ` Jeff King
  2025-06-05 22:51         ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2025-06-05 22:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Thu, Jun 05, 2025 at 09:04:15AM -0700, Junio C Hamano wrote:

> --- >8 ---
> From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Date: Thu, 5 Jun 2025 12:57:35 +0200
> Subject: [PATCH] curl: pass `long` values where expected
> 
> A set of patches posted by Jeff King earlier covered some fallouts
> coming from new typecheck warnings cURL 8.14.0.  Here are to fix
> some more instances of the same new compile errors observed in the
> `osx-gcc` job of Git's CI builds.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thanks, this patch looks good, and I think applying on top is a bit less
work. I don't mind integrating them appropriately and re-rolling if we
prefer a slightly cleaner history, though. (I don't think there's much
value in recording which hit macOS and which did not).

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-05 22:49       ` Jeff King
@ 2025-06-05 22:51         ` Jeff King
  2025-06-05 23:13           ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2025-06-05 22:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git

On Thu, Jun 05, 2025 at 06:49:10PM -0400, Jeff King wrote:

> On Thu, Jun 05, 2025 at 09:04:15AM -0700, Junio C Hamano wrote:
> 
> > --- >8 ---
> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > Date: Thu, 5 Jun 2025 12:57:35 +0200
> > Subject: [PATCH] curl: pass `long` values where expected
> > 
> > A set of patches posted by Jeff King earlier covered some fallouts
> > coming from new typecheck warnings cURL 8.14.0.  Here are to fix
> > some more instances of the same new compile errors observed in the
> > `osx-gcc` job of Git's CI builds.
> > 
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> Thanks, this patch looks good, and I think applying on top is a bit less
> work. I don't mind integrating them appropriately and re-rolling if we
> prefer a slightly cleaner history, though. (I don't think there's much
> value in recording which hit macOS and which did not).

Ah, nevermind, my patches are already in next, so building on top is
definitely best.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
  2025-06-05 22:51         ` Jeff King
@ 2025-06-05 23:13           ` Junio C Hamano
  0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2025-06-05 23:13 UTC (permalink / raw)
  To: Jeff King; +Cc: Johannes Schindelin, git

Jeff King <peff@peff.net> writes:

> On Thu, Jun 05, 2025 at 06:49:10PM -0400, Jeff King wrote:
>
>> On Thu, Jun 05, 2025 at 09:04:15AM -0700, Junio C Hamano wrote:
>> 
>> > --- >8 ---
>> > From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>> > Date: Thu, 5 Jun 2025 12:57:35 +0200
>> > Subject: [PATCH] curl: pass `long` values where expected
>> > 
>> > A set of patches posted by Jeff King earlier covered some fallouts
>> > coming from new typecheck warnings cURL 8.14.0.  Here are to fix
>> > some more instances of the same new compile errors observed in the
>> > `osx-gcc` job of Git's CI builds.
>> > 
>> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> 
>> Thanks, this patch looks good, and I think applying on top is a bit less
>> work. I don't mind integrating them appropriately and re-rolling if we
>> prefer a slightly cleaner history, though. (I don't think there's much
>> value in recording which hit macOS and which did not).

Yeah, other than giving a quick access to the places that only broke
macOS for those who are curious enough and want to find out why ;-)

> Ah, nevermind, my patches are already in next, so building on top is
> definitely best.

Yeah, in any case, taking all four of your patches together, with
Dscho's t5410 "does tee hang?" fix, finally lets the tip of 'seen'
pass without forcing retests.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-06-05 23:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-04 20:55 [PATCH 0/3] silencing warnings with curl 8.14 Jeff King
2025-06-04 20:55 ` [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt() Jeff King
2025-06-05 10:57   ` Johannes Schindelin
2025-06-05 16:04     ` Junio C Hamano
2025-06-05 22:49       ` Jeff King
2025-06-05 22:51         ` Jeff King
2025-06-05 23:13           ` Junio C Hamano
2025-06-05 22:47     ` Jeff King
2025-06-04 20:55 ` [PATCH 2/3] curl: fix integer variable " Jeff King
2025-06-04 20:56 ` [PATCH 3/3] curl: fix symbolic constant " Jeff King
2025-06-04 21:25   ` Junio C Hamano
2025-06-05  6:13   ` Daniel Stenberg
2025-06-05  7:25     ` Jeff King
2025-06-04 21:23 ` [PATCH 0/3] silencing warnings with curl 8.14 Collin Funk
2025-06-04 22:03 ` Ramsay Jones
2025-06-05  5:21 ` Patrick Steinhardt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).