Web lists-archives.com

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




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

> Jeff King <peff@xxxxxxxx> 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@xxxxxxxxxxx/

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