* [PATCH 1/3] http: offer to cast `size_t` to `curl_off_t` safely
2025-09-21 12:22 [PATCH 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
@ 2025-09-21 12:22 ` Johannes Schindelin via GitGitGadget
2025-09-21 14:52 ` Junio C Hamano
2025-09-21 12:22 ` [PATCH 2/3] imap-send: be more careful when casting to `curl_off_t` Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-21 12:22 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This commit moves the `xcurl_off_t()` function, which validates that a
given value fits within the `curl_off_t` data type and then casts it, to
a more central place so that it can be used outside of `remote-curl.c`,
too.
At the same time, this function is renamed to conform better with the
naming convention of the helper functions that safely cast from one data
type to another which has been well established in `git-compat-util.h`.
With this move, the error message can unfortunately no longer be renamed
because the `_(...)` function is not available at the time of
definition.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
http.h | 10 ++++++++++
remote-curl.c | 14 +++-----------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/http.h b/http.h
index 36202139f4..0a36dbd294 100644
--- a/http.h
+++ b/http.h
@@ -8,6 +8,7 @@ struct packed_git;
#include <curl/curl.h>
#include <curl/easy.h>
+#include "gettext.h"
#include "strbuf.h"
#include "remote.h"
@@ -95,6 +96,15 @@ static inline int missing__target(int code, int result)
#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
+static inline curl_off_t cast_size_t_to_curl_off_t(size_t a)
+{
+ uintmax_t size = a;
+ if (size > maximum_signed_value_of_type(curl_off_t))
+ die(_("number too large to represent as curl_off_t "
+ "on this platform: %"PRIuMAX), (uintmax_t)a);
+ return (curl_off_t)a;
+}
+
/*
* Normalize curl results to handle CURL_FAILONERROR (or lack thereof). Failing
* http codes have their "result" converted to CURLE_HTTP_RETURNED_ERROR, and
diff --git a/remote-curl.c b/remote-curl.c
index 84f4694780..69f919454a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -894,14 +894,6 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
return err;
}
-static curl_off_t xcurl_off_t(size_t len)
-{
- uintmax_t size = len;
- if (size > maximum_signed_value_of_type(curl_off_t))
- die(_("cannot handle pushes this big"));
- return (curl_off_t)size;
-}
-
/*
* If flush_received is true, do not attempt to read any more; just use what's
* in rpc->buf.
@@ -999,7 +991,7 @@ retry:
* and we just need to send it.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
- curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(gzip_size));
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(gzip_size));
} else if (use_gzip && 1024 < rpc->len) {
/* The client backend isn't giving us compressed data so
@@ -1030,7 +1022,7 @@ retry:
headers = curl_slist_append(headers, "Content-Encoding: gzip");
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
- curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(gzip_size));
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(gzip_size));
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (gzip %lu to %lu bytes)\n",
@@ -1043,7 +1035,7 @@ retry:
* more normal Content-Length approach.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, rpc->buf);
- curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(rpc->len));
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(rpc->len));
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (%lu bytes)\n",
rpc->service_name, (unsigned long)rpc->len);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/3] http: offer to cast `size_t` to `curl_off_t` safely
2025-09-21 12:22 ` [PATCH 1/3] http: offer to cast `size_t` to `curl_off_t` safely Johannes Schindelin via GitGitGadget
@ 2025-09-21 14:52 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2025-09-21 14:52 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> This commit moves the `xcurl_off_t()` function, which validates that a
> given value fits within the `curl_off_t` data type and then casts it, to
> a more central place so that it can be used outside of `remote-curl.c`,
> too.
>
> At the same time, this function is renamed to conform better with the
> naming convention of the helper functions that safely cast from one data
> type to another which has been well established in `git-compat-util.h`.
OK. The code inside the renamed function is the same with an
updated message to show the value.
> With this move, the error message can unfortunately no longer be renamed
> because the `_(...)` function is not available at the time of
> definition.
It is not clear to me what change (or lack thereof?) in this patch
this paragraph refers to. Who wants to rename what error message
and why?
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> http.h | 10 ++++++++++
> remote-curl.c | 14 +++-----------
> 2 files changed, 13 insertions(+), 11 deletions(-)
Thanks; will queue.
> +static inline curl_off_t cast_size_t_to_curl_off_t(size_t a)
> +{
> + uintmax_t size = a;
> + if (size > maximum_signed_value_of_type(curl_off_t))
> + die(_("number too large to represent as curl_off_t "
> + "on this platform: %"PRIuMAX), (uintmax_t)a);
> + return (curl_off_t)a;
> +}
> +
> -static curl_off_t xcurl_off_t(size_t len)
> -{
> - uintmax_t size = len;
> - if (size > maximum_signed_value_of_type(curl_off_t))
> - die(_("cannot handle pushes this big"));
> - return (curl_off_t)size;
> -}
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] imap-send: be more careful when casting to `curl_off_t`
2025-09-21 12:22 [PATCH 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
2025-09-21 12:22 ` [PATCH 1/3] http: offer to cast `size_t` to `curl_off_t` safely Johannes Schindelin via GitGitGadget
@ 2025-09-21 12:22 ` Johannes Schindelin via GitGitGadget
2025-09-21 15:06 ` Junio C Hamano
2025-09-21 12:22 ` [PATCH 3/3] http-push: avoid new compile error Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
3 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-21 12:22 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When casting a `size_t` to `curl_off_t`, there is a currently uncommon
chance that the value can be cut off (`curl_off_t` is supposed to be
guaranteed to be 64-bit).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
imap-send.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/imap-send.c b/imap-send.c
index 4bd5b8aa0d..26dda7f328 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1721,7 +1721,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
lf_to_crlf(&msgbuf.buf);
curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
- (curl_off_t)(msgbuf.buf.len-prev_len));
+ cast_size_t_to_curl_off_t(msgbuf.buf.len-prev_len));
res = curl_easy_perform(curl);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] imap-send: be more careful when casting to `curl_off_t`
2025-09-21 12:22 ` [PATCH 2/3] imap-send: be more careful when casting to `curl_off_t` Johannes Schindelin via GitGitGadget
@ 2025-09-21 15:06 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2025-09-21 15:06 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When casting a `size_t` to `curl_off_t`, there is a currently uncommon
> chance that the value can be cut off (`curl_off_t` is supposed to be
> guaranteed to be 64-bit).
"64-bit" -> "signed 64-bit", per what <curl/system.h> says,
i.e. "curl_off_t MUST be typedef'ed to a 64-bit * wide signed
integral data type.", perhaps?
msgbuf.buf is a strbuf, so its .len member is of size_t (so is
prev_len); prev_len is how big the msgbuf.buf was before the code
added some stuff taken from all_msgs, possibly wrapping the result
in html, and converting lf to crlf, all only enlarging the buffer.
We are computing how much we bloated the msgbuf.buf relative to the
original here, so this subtraction should not suffer unsigned
wraparound. Now thanks to the previous step, we can consistently
and safely cast size_t values to curl_off_t easily, which is nice.
Looking good. Will queue. Thanks.
> diff --git a/imap-send.c b/imap-send.c
> index 4bd5b8aa0d..26dda7f328 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1721,7 +1721,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
> lf_to_crlf(&msgbuf.buf);
>
> curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
> - (curl_off_t)(msgbuf.buf.len-prev_len));
> + cast_size_t_to_curl_off_t(msgbuf.buf.len-prev_len));
>
> res = curl_easy_perform(curl);
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] http-push: avoid new compile error
2025-09-21 12:22 [PATCH 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
2025-09-21 12:22 ` [PATCH 1/3] http: offer to cast `size_t` to `curl_off_t` safely Johannes Schindelin via GitGitGadget
2025-09-21 12:22 ` [PATCH 2/3] imap-send: be more careful when casting to `curl_off_t` Johannes Schindelin via GitGitGadget
@ 2025-09-21 12:22 ` Johannes Schindelin via GitGitGadget
2025-09-21 15:09 ` Junio C Hamano
2025-09-26 10:32 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
3 siblings, 1 reply; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-21 12:22 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
With the recent update in Git for Windows/ARM64 as of
https://github.com/git-for-windows/git-sdk-arm64/commit/21b288e16358
cURL was updated from v8.15.0 to v8.16.0, and the LLVM-based builds (but
strangely not the GCC-based builds) continuously greet me thusly:
http-push.c:211:2: error: call to '_curl_easy_setopt_err_long' declared
with 'warning' attribute: curl_easy_setopt expects a long argument
[-Werror,-Wattribute-warning]
CC builtin/apply.o
211 | curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
| ^
C:/a/git-sdk-arm64/git-sdk-arm64/minimal-sdk/clangarm64/include/curl/typecheck-gcc.h:50:15:
note: expanded from macro 'curl_easy_setopt'
50 | _curl_easy_setopt_err_long(); \
| ^
1 error generated.
make: *** [Makefile:2877: http-push.o] Error 1
The easiest way to shut up that compile error (which is legitimate,
seeing as the `CURLOPT_INFILESIZE` options expects a `long` parameter,
but `buffer->buf.len` refers to the `size_t` attribute of a `strbuf`)
would be to simply cast the parameter to a `long`.
However, there is a much better solution: To use the
`CURLOPT_INFILESIZE_LARGE` option instead, which was added in cURL
v7.11.0 (see https://curl.se/ch/7.11.0.html) and which Git _already_
uses in `curl_append_msgs_to_imap()`.
This fix was the motivation for renaming `xcurl_off_t()` to
`cast_size_t_to_curl_off_t()` and making it available more broadly,
which is the reason why it is used here, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
http-push.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/http-push.c b/http-push.c
index 91a5465afb..7a9b96a6d0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -208,7 +208,8 @@ static void curl_setup_http(CURL *curl, const char *url,
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);
+ curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
+ cast_size_t_to_curl_off_t(buffer->buf.len));
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/3] http-push: avoid new compile error
2025-09-21 12:22 ` [PATCH 3/3] http-push: avoid new compile error Johannes Schindelin via GitGitGadget
@ 2025-09-21 15:09 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2025-09-21 15:09 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> However, there is a much better solution: To use the
> `CURLOPT_INFILESIZE_LARGE` option instead, which was added in cURL
> v7.11.0 (see https://curl.se/ch/7.11.0.html) and which Git _already_
> uses in `curl_append_msgs_to_imap()`.
>
> This fix was the motivation for renaming `xcurl_off_t()` to
> `cast_size_t_to_curl_off_t()` and making it available more broadly,
> which is the reason why it is used here, too.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> http-push.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
As we saw in [1/3], we have been using "size large" for posting in
remote-curl.c and this brings how the size for reading the data is
handled to match that.
Looking good.
Will queue. Thanks.
> diff --git a/http-push.c b/http-push.c
> index 91a5465afb..7a9b96a6d0 100644
> --- a/http-push.c
> +++ b/http-push.c
> @@ -208,7 +208,8 @@ static void curl_setup_http(CURL *curl, const char *url,
> 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);
> + curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
> + cast_size_t_to_curl_off_t(buffer->buf.len));
> curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
> curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
> curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again
2025-09-21 12:22 [PATCH 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2025-09-21 12:22 ` [PATCH 3/3] http-push: avoid new compile error Johannes Schindelin via GitGitGadget
@ 2025-09-26 10:32 ` Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 1/3] http: offer to cast `size_t` to `curl_off_t` safely Johannes Schindelin via GitGitGadget
` (3 more replies)
3 siblings, 4 replies; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-26 10:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
As of last week, every CI build of Git for Windows' ARM64 flavor of its SDK
started failing (the first failed build is this here:
https://github.com/git-for-windows/git-sdk-arm64/actions/runs/17633130672/job/50104373185).
The reason is that we still have one curl_easy_setopt() call that is
supposed to pass a long parameter but doesn't.
The likely explanation is an update to libcurl in the Git for Windows/ARM64
SDK. A strange thing about this: GCC does not catch the problem, but clang
does. And even more strangely, the relevant parts of cURL's typecheck-gcc.h
haven't changed in a way that explains to me why this starts to trigger now.
To prevent more piecemeal solutions, I vetted all calls of
curl_easy_setopt() and satisified myself that all remaining calls that want
long parameters do indeed have arguments matching that type.
As is the custom in the Git project, I use this opportunity to do a bit more
than is strictly necessary to address this problem. I replace the affected
call with one that expects a wider data type, refactor an already-existing
function to cast to said data type, and use it in the modified call as well
as in another call which wants to perform the same cast but did so unsafely
(not that it mattered in practice, really, yet it is in line with other
refactors the Git project has seen).
Note for the curious: The affected call used CURLOPT_INFILESIZE
[https://curl.se/libcurl/c/CURLOPT_INFILESIZE.html] and now uses
CURLOPT_INFILESIZE_LARGE
[https://curl.se/libcurl/c/CURLOPT_INFILESIZE_LARGE.html], and it is a bit
strange that the latter wasn't used in the first place, given that it was
introduced into cURL over a year before Git itself was born, and almost two
years before the call was introduced in 58e60dd2033 (Add support for pushing
to a remote repository using HTTP/DAV, 2005-11-02).
For the record, these patches apply cleanly all the way back to v2.22,
according to git replay. I did not try to test whether it builds, though,
because all kinds of stunts are required nowadays to build this old versions
even without any patches on top.
Changes since v1:
* The commit messages now reflect Junio's suggested edits.
Johannes Schindelin (3):
http: offer to cast `size_t` to `curl_off_t` safely
imap-send: be more careful when casting to `curl_off_t`
http-push: avoid new compile error
http-push.c | 3 ++-
http.h | 10 ++++++++++
imap-send.c | 2 +-
remote-curl.c | 14 +++-----------
4 files changed, 16 insertions(+), 13 deletions(-)
base-commit: ca2559c1d630eb4f04cdee2328aaf1c768907a9e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1974%2Fdscho%2Fcurl-easy-setopt-typefix-again-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1974/dscho/curl-easy-setopt-typefix-again-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1974
Range-diff vs v1:
1: 7caaec9102 ! 1: 114eb9c8ec http: offer to cast `size_t` to `curl_off_t` safely
@@ Commit message
naming convention of the helper functions that safely cast from one data
type to another which has been well established in `git-compat-util.h`.
- With this move, the error message can unfortunately no longer be renamed
- because the `_(...)` function is not available at the time of
- definition.
+ With this move, `gettext.h` must be `#include`d in `http.h` to allow the
+ error message to remain translatable.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
2: e1d5a85f70 ! 2: fdd1327e7b imap-send: be more careful when casting to `curl_off_t`
@@ Commit message
imap-send: be more careful when casting to `curl_off_t`
When casting a `size_t` to `curl_off_t`, there is a currently uncommon
- chance that the value can be cut off (`curl_off_t` is supposed to be
- guaranteed to be 64-bit).
+ chance that the value can be cut off (`curl_off_t` is expected to be a
+ signed 64-bit data type).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
3: bcb231125c = 3: 140755673d http-push: avoid new compile error
--
gitgitgadget
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/3] http: offer to cast `size_t` to `curl_off_t` safely
2025-09-26 10:32 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
@ 2025-09-26 10:32 ` Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 2/3] imap-send: be more careful when casting to `curl_off_t` Johannes Schindelin via GitGitGadget
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-26 10:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
This commit moves the `xcurl_off_t()` function, which validates that a
given value fits within the `curl_off_t` data type and then casts it, to
a more central place so that it can be used outside of `remote-curl.c`,
too.
At the same time, this function is renamed to conform better with the
naming convention of the helper functions that safely cast from one data
type to another which has been well established in `git-compat-util.h`.
With this move, `gettext.h` must be `#include`d in `http.h` to allow the
error message to remain translatable.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
http.h | 10 ++++++++++
remote-curl.c | 14 +++-----------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/http.h b/http.h
index 36202139f4..0a36dbd294 100644
--- a/http.h
+++ b/http.h
@@ -8,6 +8,7 @@ struct packed_git;
#include <curl/curl.h>
#include <curl/easy.h>
+#include "gettext.h"
#include "strbuf.h"
#include "remote.h"
@@ -95,6 +96,15 @@ static inline int missing__target(int code, int result)
#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
+static inline curl_off_t cast_size_t_to_curl_off_t(size_t a)
+{
+ uintmax_t size = a;
+ if (size > maximum_signed_value_of_type(curl_off_t))
+ die(_("number too large to represent as curl_off_t "
+ "on this platform: %"PRIuMAX), (uintmax_t)a);
+ return (curl_off_t)a;
+}
+
/*
* Normalize curl results to handle CURL_FAILONERROR (or lack thereof). Failing
* http codes have their "result" converted to CURLE_HTTP_RETURNED_ERROR, and
diff --git a/remote-curl.c b/remote-curl.c
index 84f4694780..69f919454a 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -894,14 +894,6 @@ static int probe_rpc(struct rpc_state *rpc, struct slot_results *results)
return err;
}
-static curl_off_t xcurl_off_t(size_t len)
-{
- uintmax_t size = len;
- if (size > maximum_signed_value_of_type(curl_off_t))
- die(_("cannot handle pushes this big"));
- return (curl_off_t)size;
-}
-
/*
* If flush_received is true, do not attempt to read any more; just use what's
* in rpc->buf.
@@ -999,7 +991,7 @@ retry:
* and we just need to send it.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
- curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(gzip_size));
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(gzip_size));
} else if (use_gzip && 1024 < rpc->len) {
/* The client backend isn't giving us compressed data so
@@ -1030,7 +1022,7 @@ retry:
headers = curl_slist_append(headers, "Content-Encoding: gzip");
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);
- curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(gzip_size));
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(gzip_size));
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (gzip %lu to %lu bytes)\n",
@@ -1043,7 +1035,7 @@ retry:
* more normal Content-Length approach.
*/
curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, rpc->buf);
- curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, xcurl_off_t(rpc->len));
+ curl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, cast_size_t_to_curl_off_t(rpc->len));
if (options.verbosity > 1) {
fprintf(stderr, "POST %s (%lu bytes)\n",
rpc->service_name, (unsigned long)rpc->len);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 2/3] imap-send: be more careful when casting to `curl_off_t`
2025-09-26 10:32 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 1/3] http: offer to cast `size_t` to `curl_off_t` safely Johannes Schindelin via GitGitGadget
@ 2025-09-26 10:32 ` Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 3/3] http-push: avoid new compile error Johannes Schindelin via GitGitGadget
2025-09-26 17:40 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Junio C Hamano
3 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-26 10:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When casting a `size_t` to `curl_off_t`, there is a currently uncommon
chance that the value can be cut off (`curl_off_t` is expected to be a
signed 64-bit data type).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
imap-send.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/imap-send.c b/imap-send.c
index 4bd5b8aa0d..26dda7f328 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1721,7 +1721,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,
lf_to_crlf(&msgbuf.buf);
curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
- (curl_off_t)(msgbuf.buf.len-prev_len));
+ cast_size_t_to_curl_off_t(msgbuf.buf.len-prev_len));
res = curl_easy_perform(curl);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] http-push: avoid new compile error
2025-09-26 10:32 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 1/3] http: offer to cast `size_t` to `curl_off_t` safely Johannes Schindelin via GitGitGadget
2025-09-26 10:32 ` [PATCH v2 2/3] imap-send: be more careful when casting to `curl_off_t` Johannes Schindelin via GitGitGadget
@ 2025-09-26 10:32 ` Johannes Schindelin via GitGitGadget
2025-09-26 17:40 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Junio C Hamano
3 siblings, 0 replies; 12+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2025-09-26 10:32 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
With the recent update in Git for Windows/ARM64 as of
https://github.com/git-for-windows/git-sdk-arm64/commit/21b288e16358
cURL was updated from v8.15.0 to v8.16.0, and the LLVM-based builds (but
strangely not the GCC-based builds) continuously greet me thusly:
http-push.c:211:2: error: call to '_curl_easy_setopt_err_long' declared
with 'warning' attribute: curl_easy_setopt expects a long argument
[-Werror,-Wattribute-warning]
CC builtin/apply.o
211 | curl_easy_setopt(curl, CURLOPT_INFILESIZE, buffer->buf.len);
| ^
C:/a/git-sdk-arm64/git-sdk-arm64/minimal-sdk/clangarm64/include/curl/typecheck-gcc.h:50:15:
note: expanded from macro 'curl_easy_setopt'
50 | _curl_easy_setopt_err_long(); \
| ^
1 error generated.
make: *** [Makefile:2877: http-push.o] Error 1
The easiest way to shut up that compile error (which is legitimate,
seeing as the `CURLOPT_INFILESIZE` options expects a `long` parameter,
but `buffer->buf.len` refers to the `size_t` attribute of a `strbuf`)
would be to simply cast the parameter to a `long`.
However, there is a much better solution: To use the
`CURLOPT_INFILESIZE_LARGE` option instead, which was added in cURL
v7.11.0 (see https://curl.se/ch/7.11.0.html) and which Git _already_
uses in `curl_append_msgs_to_imap()`.
This fix was the motivation for renaming `xcurl_off_t()` to
`cast_size_t_to_curl_off_t()` and making it available more broadly,
which is the reason why it is used here, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
http-push.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/http-push.c b/http-push.c
index 91a5465afb..7a9b96a6d0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -208,7 +208,8 @@ static void curl_setup_http(CURL *curl, const char *url,
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);
+ curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE,
+ cast_size_t_to_curl_off_t(buffer->buf.len));
curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
curl_easy_setopt(curl, CURLOPT_SEEKFUNCTION, seek_buffer);
curl_easy_setopt(curl, CURLOPT_SEEKDATA, buffer);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again
2025-09-26 10:32 ` [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again Johannes Schindelin via GitGitGadget
` (2 preceding siblings ...)
2025-09-26 10:32 ` [PATCH v2 3/3] http-push: avoid new compile error Johannes Schindelin via GitGitGadget
@ 2025-09-26 17:40 ` Junio C Hamano
3 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2025-09-26 17:40 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> As of last week, every CI build of Git for Windows' ARM64 flavor of its SDK
> started failing (the first failed build is this here:
> https://github.com/git-for-windows/git-sdk-arm64/actions/runs/17633130672/job/50104373185).
> ..
> For the record, these patches apply cleanly all the way back to v2.22,
> according to git replay. I did not try to test whether it builds, though,
> because all kinds of stunts are required nowadays to build this old versions
> even without any patches on top.
> Range-diff vs v1:
>
> 1: 7caaec9102 ! 1: 114eb9c8ec http: offer to cast `size_t` to `curl_off_t` safely
> @@ Commit message
> naming convention of the helper functions that safely cast from one data
> type to another which has been well established in `git-compat-util.h`.
>
> - With this move, the error message can unfortunately no longer be renamed
> - because the `_(...)` function is not available at the time of
> - definition.
> + With this move, `gettext.h` must be `#include`d in `http.h` to allow the
> + error message to remain translatable.
Ahh, OK. Now I understand that's what you meant.
> 2: e1d5a85f70 ! 2: fdd1327e7b imap-send: be more careful when casting to `curl_off_t`
> @@ Commit message
> imap-send: be more careful when casting to `curl_off_t`
>
> When casting a `size_t` to `curl_off_t`, there is a currently uncommon
> - chance that the value can be cut off (`curl_off_t` is supposed to be
> - guaranteed to be 64-bit).
> + chance that the value can be cut off (`curl_off_t` is expected to be a
> + signed 64-bit data type).
Will replace. Let me mark the topic for 'next'.
Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread