RE: [PATCH] http.postbuffer: make a size_t

[David Turner <David.Turner@twosigma.com>]

> -----Original Message-----
> From: Torsten Bögershausen [mailto:tboegi@web.de]
> Sent: Friday, March 31, 2017 12:22 AM
> To: David Turner <David.Turner@twosigma.com>; git@vger.kernel.org
> Subject: Re: [PATCH] http.postbuffer: make a size_t
> 
> 
> 
> On 30/03/17 22:29, David Turner wrote:
> > Unfortunately, in order to push some large repos, the http postbuffer
> > must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:
> > we just malloc a larger buffer.
> >
> > Signed-off-by: David Turner <dturner@twosigma.com>
> > ---
> >  cache.h  |  1 +
> >  config.c | 17 +++++++++++++++++
> >  http.c   |  2 +-
> >  3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/cache.h b/cache.h
> > index fbdf7a815a..a8c1b65db0 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1900,6 +1900,7 @@ extern int git_parse_maybe_bool(const char *);
> > extern int git_config_int(const char *, const char *);  extern int64_t
> > git_config_int64(const char *, const char *);  extern unsigned long
> > git_config_ulong(const char *, const char *);
> > +extern size_t git_config_size_t(const char *, const char *);
> >  extern int git_config_bool_or_int(const char *, const char *, int *);
> > extern int git_config_bool(const char *, const char *);  extern int
> > git_config_maybe_bool(const char *, const char *); diff --git
> > a/config.c b/config.c index 1a4d85537b..7b706cf27a 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -834,6 +834,15 @@ int git_parse_ulong(const char *value, unsigned
> long *ret)
> >  	return 1;
> >  }
> >
> > +static size_t git_parse_size_t(const char *value, unsigned long *ret)
> > +{
> > +	size_t tmp;
> > +	if (!git_parse_signed(value, &tmp,
> maximum_unsigned_value_of_type(size_t)))
> > +		return 0;
> > +	*ret = tmp;
> > +	return 1;
> > +}
> What is the return value here ?
> Isn't it a size_t we want ?

Yeah, that should return an int, since it's just "parsed" vs "unparsed".

> (There was a recent discussion about "unsigned long" vs "size_t", which
>   are the same on many systems, but not under Win64) Would the following
> work ?
> 
> static int git_parse_size_t(const char *value, size_t *ret) {
> 	if (!git_parse_signed(value, ret,
> maximum_unsigned_value_of_type(size_t)))
> 		return 0;
> 	return 1;
> }
> 
> []
> > +size_t git_config_size_t(const char *name, const char *value) {
> > +	unsigned long ret;
> > +	if (!git_parse_size_t(value, &ret))
> > +		die_bad_number(name, value);
> > +	return ret;
> > +}
> > +
> Same here:
> size_t git_config_size_t(const char *name, const char *value) {
> 	size_t ret;
> 	if (!git_parse_size_t(value, &ret))
> 		die_bad_number(name, value);
> 	return ret;
> }

In v2 I decided to make this a ssize_t, since it was previously a signed int.  I know we're not using negative values right now, but since nobody has 2**63 bytes of ram anyway, it's probably safe to keep it signed to keep our options open.