Re: [PATCH v2 4/4] curl: pass `long` values where expected

From: Jeff King <peff@peff.net>
Date: Jun 6, 2025, 10:05 UTC
Message-ID: <20250606100558.GA3591871@coredump.intra.peff.net>
In-Reply-To: <80de7491d24fb51c6b2c3b2fc1728db30e2477f7.1749202164.git.gitgitgadget@gmail.com>

On Fri, Jun 06, 2025 at 09:29:24AM +0000, Johannes Schindelin via GitGitGadget wrote:

Show 9 quoted lines

> Nearly identical compile errors afflicted recently-updated Debian
> setups, which have been addressed by `jk/curl-easy-setopt-typefix`.
> 
> However, on macOS Git is built with different build options, which
> uncovered more instances of `int` values that need to be cast to
> constants, which were not covered by 6f11c42e8edc (curl: fix integer
> constant typechecks with curl_easy_setopt(), 2025-06-04). Let's
> explicitly convert even those remaining `int` constants in
> `curl_easy_setopt()` calls to `long` parameters.

What different build options? The extra fixes are in code that is compiled on all platforms. E.g.:

Show 12 quoted lines

> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -970,8 +970,8 @@ retry:
>  
>  	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, "");

So I think the root cause of the difference remains a mystery.

Curiously, if I build libcurl from source and link against it on my Debian system, I get all of the errors (including the ones you're fixing here). But with the exact same compile options (modulo pointing CURL_CONFIG at the right spot), building against the Debian-packaged libcurl is OK. Weird.

Show 11 quoted lines

> In addition to looking at the compile errors of the `osx-gcc` job, I
> verified that there are no other instances of the same issue that need
> to be handled in this manner (and that might not be caught by our CI
> builds because of yet other build options that might skip those code
> parts), I ran the following command and inspected all 23 results
> manually to ensure that the fix is now actually complete:
> 
>   git grep -n curl_easy_setopt |
>   grep -ve ',.*, *[A-Za-z_"&]' \
>     -e ',.*, *[-0-9]*L)' \
>     -e ',.*,.* (long)'

I don't think that's sufficient for a full audit, because the first "grep -v" regex is removing variable names and symbolic constants, which might also need to be cast to long (i.e., my patches 2 and 3).

But as your patch fixes the exact set that I also needed when building against my custom-built libcurl, I'm content to say we have spent enough time digging. If there is some platform or makefile knob combination that triggers one we missed, then curl's type-checker will catch it and we can fix it then.

-Peff