Re: [PATCH v6] http.postbuffer: allow full range of ssize_t values

From: Jeff King <peff@peff.net>
Date: Apr 12, 2017, 12:52 UTC
Message-ID: <20170412125214.uiex4ludmgahsvme@sigill.intra.peff.net>
In-Reply-To: <xmqqa87mqrhc.fsf@gitster.mtv.corp.google.com>

On Tue, Apr 11, 2017 at 07:39:27PM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> >> The only unresolved issue was whether we can count on curl being new
> >> enough for CURLOPT_POSTFIELDSIZE_LARGE to be present.  I say
> >> "unresolved" but it is resolved in my mind since git doesn't build and
> >> pass tests with such old versions of curl --- what's unresolved is
> >> formalizing what the oldest curl version is that we want to support.
> >> And that doesn't need to hold this patch hostage.
> >
> > It could build on older curl with a minor fix; the regression is in
> > v2.12. So if we did want to continue to support the same versions of
> > curl we did in v2.11, we could apply that fix and then we _would_ care
> > about #ifdef-ing this.
> 
> What would the fix be?  Have a code that notices that the value set
> to http.postbuffer is too large and ignore the request on the other
> side of #ifdef, i.e. when Git is built with older curl that lack
> CURLOPT_POSTFIELDSIZE_LARGE?

The fix I meant there is for a different spot. During the course of the discussion, somebody noticed that v2.12 does not compile using older curls:

  http://public-inbox.org/git/20170404133241.GA15588@gevaerts.be/

So _if_ we care about those older curls, then we should consider that a regression and fix it on the v2.12-maint track.

And likewise, we should not accept this patch into master without a similar fix. Which is probably, yes, an ifdef for older curl that uses the non-LARGE version of POSTFIELDSIZE and defines xcurl_off_t() to check against "long" and complain when it overflows.

-Peff