Skip to content

Fix curl_easy_setopt() parameter type problem, again

Johannes Schindelin via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest3 patches 1 replyIn review
  2. v13 patches 3 repliesSuperseded

Changes in v2 from the cover letter

* The commit messages now reflect Junio's suggested edits.

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/3http: offer to cast `size_t` to `curl_off_t` safelyhttp.h remote-curl.c+13 -110 replies
2/3imap-send: be more careful when casting to `curl_off_t`imap-send.c+1 -10 replies
3/3http-push: avoid new compile errorhttp-push.c+2 -10 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/3] Fix curl_easy_setopt() parameter type problem, again

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 #included in http.h to allow the
+ error message to remain translatable.

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

3: bcb231125c = 3: 140755673d http-push: avoid new compile error

Signature · 1 line
gitgitgadget