Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jun 5, 2025, 10:57 UTC
- Message-ID
- <9bd5f0f3-d0c5-067b-ffa6-12a2c0353580@gmx.de>
- In-Reply-To
- <20250604205513.GA1510819@coredump.intra.peff.net>
Hi Jeff,
On Wed, 4 Jun 2025, Jeff King wrote:
Show 10 quoted lines
> 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.
Show 7 quoted lines
> 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