Re: [PATCH 1/3] http-push: prefer CURLOPT_UPLOAD to CURLOPT_PUT

From: Jeff King <peff@peff.net>
Date: Jan 15, 2023, 23:49 UTC
Message-ID: <Y8SQ9ua7w2MbeU9C@coredump.intra.peff.net>
In-Reply-To: <Y8SIf8AyGrNflBA7@coredump.intra.peff.net>

On Sun, Jan 15, 2023 at 06:13:04PM -0500, Jeff King wrote:

Show 15 quoted lines

> On Sun, Jan 15, 2023 at 08:54:59PM +0000, Ramsay Jones wrote:
> 
> > > -	curl_easy_setopt(curl, CURLOPT_PUT, 1);
> > > +	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1);
> > 
> > My version of this patch had '1L' rather than just '1' - but it
> > doesn't really matter (and was probably because all the curl
> > examples did so!).
> 
> Yeah, I think it probably ought to be 1L because it's going to a
> variadic function (and it's been a while, but I think the regular
> integer promotions make small things into int, but not into long).
> 
> So I don't mind fixing it, but I think we'd want it in a separate patch,
> since it's orthogonal to what's happening here.

I dug a little bit more and as far as I can tell, yes, "1" is technically violating the standard but will often work depending on the ABI and endianness of the platform.

That said, there are other cases besides this one, too, so if we want to tackle it, let's make it a separate topic (but I'm happy to leave it if nobody's platform is broken).

-Peff