Re: [PATCH 1/3] curl: fix integer constant typechecks with curl_easy_setopt()

From: Jeff King <peff@peff.net>
Date: Jun 5, 2025, 22:47 UTC
Message-ID: <20250605224747.GA3005733@coredump.intra.peff.net>
In-Reply-To: <9bd5f0f3-d0c5-067b-ffa6-12a2c0353580@gmx.de>

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

Show 13 quoted lines

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