{"thread":{"id":"45553","subject":"[PATCH] http.postbuffer: make a size_t","startedAt":"2017-03-30T20:29:33Z","lastAt":"2017-03-31T15:57:58Z","messageCount":6,"participants":["David Turner","Shawn Pearce","Junio C Hamano","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"315823","messageId":"20170330202917.24281-1-dturner@twosigma.com","threadId":"45553","inReplyTo":null,"subject":"[PATCH] http.postbuffer: make a size_t","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2017-03-30T20:29:17Z","receivedAt":"2017-03-30T20:29:33Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Unfortunately, in order to push some large repos, the http postbuffer\nmust sometimes exceed two gigabytes.  On a 64-bit system, this is OK:\nwe just malloc a larger buffer.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\n---\n cache.h  |  1 +\n config.c | 17 +++++++++++++++++\n http.c   |  2 +-\n 3 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/cache.h b/cache.h\nindex fbdf7a815a..a8c1b65db0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1900,6 +1900,7 @@ extern int git_parse_maybe_bool(const char *);\n extern int git_config_int(const char *, const char *);\n extern int64_t git_config_int64(const char *, const char *);\n extern unsigned long git_config_ulong(const char *, const char *);\n+extern size_t git_config_size_t(const char *, const char *);\n extern int git_config_bool_or_int(const char *, const char *, int *);\n extern int git_config_bool(const char *, const char *);\n extern int git_config_maybe_bool(const char *, const char *);\ndiff --git a/config.c b/config.c\nindex 1a4d85537b..7b706cf27a 100644\n--- a/config.c\n+++ b/config.c\n@@ -834,6 +834,15 @@ int git_parse_ulong(const char *value, unsigned long *ret)\n \treturn 1;\n }\n \n+static size_t git_parse_size_t(const char *value, unsigned long *ret)\n+{\n+\tsize_t tmp;\n+\tif (!git_parse_signed(value, &tmp, maximum_unsigned_value_of_type(size_t)))\n+\t\treturn 0;\n+\t*ret = tmp;\n+\treturn 1;\n+}\n+\n NORETURN\n static void die_bad_number(const char *name, const char *value)\n {\n@@ -892,6 +901,14 @@ unsigned long git_config_ulong(const char *name, const char *value)\n \treturn ret;\n }\n \n+size_t git_config_size_t(const char *name, const char *value)\n+{\n+\tunsigned long ret;\n+\tif (!git_parse_size_t(value, &ret))\n+\t\tdie_bad_number(name, value);\n+\treturn ret;\n+}\n+\n int git_parse_maybe_bool(const char *value)\n {\n \tif (!value)\ndiff --git a/http.c b/http.c\nindex 96d84bbed3..ab6080835f 100644\n--- a/http.c\n+++ b/http.c\n@@ -331,7 +331,7 @@ static int http_options(const char *var, const char *value, void *cb)\n \t}\n \n \tif (!strcmp(\"http.postbuffer\", var)) {\n-\t\thttp_post_buffer = git_config_int(var, value);\n+\t\thttp_post_buffer = git_config_size_t(var, value);\n \t\tif (http_post_buffer < LARGE_PACKET_MAX)\n \t\t\thttp_post_buffer = LARGE_PACKET_MAX;\n \t\treturn 0;\n-- \n2.11.GIT\n\n"},{"id":"315826","messageId":"CAJo=hJvs0UKO_NLbWAi8Y8XRJJ0v5kdjf9BN0V=pN_9yYrDp7Q@mail.gmail.com","threadId":"45553","inReplyTo":"20170330202917.24281-1-dturner@twosigma.com","subject":"Re: [PATCH] http.postbuffer: make a size_t","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2017-03-30T20:42:09Z","receivedAt":"2017-03-30T20:42:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Thu, Mar 30, 2017 at 1:29 PM, David Turner <dturner@twosigma.com> wrote:\n> Unfortunately, in order to push some large repos, the http postbuffer\n> must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:\n> we just malloc a larger buffer.\n\nI'm slightly curious what server you are pushing to that needs the\nentire thing buffered to compute a Content-Length, rather than using\nTransfer-Encoding: chunked. Most Git-over-HTTP should be accepting\nTransfer-Encoding: chunked when the stream exceeds postBuffer.\n"},{"id":"315831","messageId":"82ec49a3ae4d402daf6113cf19c47fb0@exmbdft7.ad.twosigma.com","threadId":"45553","inReplyTo":"CAJo=hJvs0UKO_NLbWAi8Y8XRJJ0v5kdjf9BN0V=pN_9yYrDp7Q@mail.gmail.com","subject":"RE: [PATCH] http.postbuffer: make a size_t","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2017-03-30T20:59:31Z","receivedAt":"2017-03-30T20:59:38Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"GitLab.  I can't speak to our particular configuration of it -- but if you have a specific question about what the config is, I can ask our gitlab admins.\n\n> -----Original Message-----\n> From: Shawn Pearce [mailto:spearce@spearce.org]\n> Sent: Thursday, March 30, 2017 4:42 PM\n> To: David Turner <David.Turner@twosigma.com>\n> Cc: git <git@vger.kernel.org>\n> Subject: Re: [PATCH] http.postbuffer: make a size_t\n> \n> On Thu, Mar 30, 2017 at 1:29 PM, David Turner <dturner@twosigma.com>\n> wrote:\n> > Unfortunately, in order to push some large repos, the http postbuffer\n> > must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:\n> > we just malloc a larger buffer.\n> \n> I'm slightly curious what server you are pushing to that needs the entire thing\n> buffered to compute a Content-Length, rather than using\n> Transfer-Encoding: chunked. Most Git-over-HTTP should be accepting\n> Transfer-Encoding: chunked when the stream exceeds postBuffer.\n"},{"id":"315840","messageId":"xmqqwpb61kre.fsf@gitster.mtv.corp.google.com","threadId":"45553","inReplyTo":"20170330202917.24281-1-dturner@twosigma.com","subject":"Re: [PATCH] http.postbuffer: make a size_t","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-30T22:07:33Z","receivedAt":"2017-03-30T22:07:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twosigma.com> writes:\n\n> Unfortunately, in order to push some large repos, the http postbuffer\n> must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:\n> we just malloc a larger buffer.\n>\n> Signed-off-by: David Turner <dturner@twosigma.com>\n> ---\n>  cache.h  |  1 +\n>  config.c | 17 +++++++++++++++++\n>  http.c   |  2 +-\n>  3 files changed, 19 insertions(+), 1 deletion(-)\n>\n> diff --git a/cache.h b/cache.h\n> index fbdf7a815a..a8c1b65db0 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1900,6 +1900,7 @@ extern int git_parse_maybe_bool(const char *);\n>  extern int git_config_int(const char *, const char *);\n>  extern int64_t git_config_int64(const char *, const char *);\n>  extern unsigned long git_config_ulong(const char *, const char *);\n> +extern size_t git_config_size_t(const char *, const char *);\n>  extern int git_config_bool_or_int(const char *, const char *, int *);\n>  extern int git_config_bool(const char *, const char *);\n>  extern int git_config_maybe_bool(const char *, const char *);\n> diff --git a/config.c b/config.c\n> index 1a4d85537b..7b706cf27a 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -834,6 +834,15 @@ int git_parse_ulong(const char *value, unsigned long *ret)\n>  \treturn 1;\n>  }\n>  \n> +static size_t git_parse_size_t(const char *value, unsigned long *ret)\n> +{\n> +\tsize_t tmp;\n> +\tif (!git_parse_signed(value, &tmp, maximum_unsigned_value_of_type(size_t)))\n\nI am getting these:\n\nconfig.c:840:2: error: pointer targets in passing argument 2 of 'git_parse_signed' differ in signedness [-Werror=pointer-sign]\n  if (!git_parse_signed(value, &tmp, maximum_unsigned_value_of_type(size_t)))\n  ^\n\nconfig.c:753:12: note: expected 'intmax_t *' but argument is of type 'size_t *'\n static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)\n            ^\n\nChanging \"size_t tmp\" to \"intmax_t tmp\" squelches it but the maximum\nunsigned value of size_t type would probably overflow \"intmax_t max\"\nwhich is signed, so...\n"},{"id":"315870","messageId":"d312abfe-f87d-3940-ecfa-348fed7ad3a1@web.de","threadId":"45553","inReplyTo":"20170330202917.24281-1-dturner@twosigma.com","subject":"Re: [PATCH] http.postbuffer: make a size_t","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-03-31T04:22:13Z","receivedAt":"2017-03-31T04:21:36Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"\n\nOn 30/03/17 22:29, David Turner wrote:\n> Unfortunately, in order to push some large repos, the http postbuffer\n> must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:\n> we just malloc a larger buffer.\n>\n> Signed-off-by: David Turner <dturner@twosigma.com>\n> ---\n>  cache.h  |  1 +\n>  config.c | 17 +++++++++++++++++\n>  http.c   |  2 +-\n>  3 files changed, 19 insertions(+), 1 deletion(-)\n>\n> diff --git a/cache.h b/cache.h\n> index fbdf7a815a..a8c1b65db0 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1900,6 +1900,7 @@ extern int git_parse_maybe_bool(const char *);\n>  extern int git_config_int(const char *, const char *);\n>  extern int64_t git_config_int64(const char *, const char *);\n>  extern unsigned long git_config_ulong(const char *, const char *);\n> +extern size_t git_config_size_t(const char *, const char *);\n>  extern int git_config_bool_or_int(const char *, const char *, int *);\n>  extern int git_config_bool(const char *, const char *);\n>  extern int git_config_maybe_bool(const char *, const char *);\n> diff --git a/config.c b/config.c\n> index 1a4d85537b..7b706cf27a 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -834,6 +834,15 @@ int git_parse_ulong(const char *value, unsigned long *ret)\n>  \treturn 1;\n>  }\n>\n> +static size_t git_parse_size_t(const char *value, unsigned long *ret)\n> +{\n> +\tsize_t tmp;\n> +\tif (!git_parse_signed(value, &tmp, maximum_unsigned_value_of_type(size_t)))\n> +\t\treturn 0;\n> +\t*ret = tmp;\n> +\treturn 1;\n> +}\nWhat is the return value here ?\nIsn't it a size_t we want ?\n(There was a recent discussion about \"unsigned long\" vs \"size_t\", which\n  are the same on many systems, but not under Win64)\nWould the following work ?\n\nstatic int git_parse_size_t(const char *value, size_t *ret)\n{\n\tif (!git_parse_signed(value, ret, maximum_unsigned_value_of_type(size_t)))\n\t\treturn 0;\n\treturn 1;\n}\n\n[]\n> +size_t git_config_size_t(const char *name, const char *value)\n> +{\n> +\tunsigned long ret;\n> +\tif (!git_parse_size_t(value, &ret))\n> +\t\tdie_bad_number(name, value);\n> +\treturn ret;\n> +}\n> +\nSame here:\nsize_t git_config_size_t(const char *name, const char *value)\n{\n\tsize_t ret;\n\tif (!git_parse_size_t(value, &ret))\n\t\tdie_bad_number(name, value);\n\treturn ret;\n}\n"},{"id":"315920","messageId":"8d9af9884ccc4f99a01da863cb7ba600@exmbdft7.ad.twosigma.com","threadId":"45553","inReplyTo":"d312abfe-f87d-3940-ecfa-348fed7ad3a1@web.de","subject":"RE: [PATCH] http.postbuffer: make a size_t","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2017-03-31T15:52:35Z","receivedAt":"2017-03-31T15:57:58Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"> -----Original Message-----\n> From: Torsten Bögershausen [mailto:tboegi@web.de]\n> Sent: Friday, March 31, 2017 12:22 AM\n> To: David Turner <David.Turner@twosigma.com>; git@vger.kernel.org\n> Subject: Re: [PATCH] http.postbuffer: make a size_t\n> \n> \n> \n> On 30/03/17 22:29, David Turner wrote:\n> > Unfortunately, in order to push some large repos, the http postbuffer\n> > must sometimes exceed two gigabytes.  On a 64-bit system, this is OK:\n> > we just malloc a larger buffer.\n> >\n> > Signed-off-by: David Turner <dturner@twosigma.com>\n> > ---\n> >  cache.h  |  1 +\n> >  config.c | 17 +++++++++++++++++\n> >  http.c   |  2 +-\n> >  3 files changed, 19 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/cache.h b/cache.h\n> > index fbdf7a815a..a8c1b65db0 100644\n> > --- a/cache.h\n> > +++ b/cache.h\n> > @@ -1900,6 +1900,7 @@ extern int git_parse_maybe_bool(const char *);\n> > extern int git_config_int(const char *, const char *);  extern int64_t\n> > git_config_int64(const char *, const char *);  extern unsigned long\n> > git_config_ulong(const char *, const char *);\n> > +extern size_t git_config_size_t(const char *, const char *);\n> >  extern int git_config_bool_or_int(const char *, const char *, int *);\n> > extern int git_config_bool(const char *, const char *);  extern int\n> > git_config_maybe_bool(const char *, const char *); diff --git\n> > a/config.c b/config.c index 1a4d85537b..7b706cf27a 100644\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -834,6 +834,15 @@ int git_parse_ulong(const char *value, unsigned\n> long *ret)\n> >  \treturn 1;\n> >  }\n> >\n> > +static size_t git_parse_size_t(const char *value, unsigned long *ret)\n> > +{\n> > +\tsize_t tmp;\n> > +\tif (!git_parse_signed(value, &tmp,\n> maximum_unsigned_value_of_type(size_t)))\n> > +\t\treturn 0;\n> > +\t*ret = tmp;\n> > +\treturn 1;\n> > +}\n> What is the return value here ?\n> Isn't it a size_t we want ?\n\nYeah, that should return an int, since it's just \"parsed\" vs \"unparsed\".\n\n> (There was a recent discussion about \"unsigned long\" vs \"size_t\", which\n>   are the same on many systems, but not under Win64) Would the following\n> work ?\n> \n> static int git_parse_size_t(const char *value, size_t *ret) {\n> \tif (!git_parse_signed(value, ret,\n> maximum_unsigned_value_of_type(size_t)))\n> \t\treturn 0;\n> \treturn 1;\n> }\n> \n> []\n> > +size_t git_config_size_t(const char *name, const char *value) {\n> > +\tunsigned long ret;\n> > +\tif (!git_parse_size_t(value, &ret))\n> > +\t\tdie_bad_number(name, value);\n> > +\treturn ret;\n> > +}\n> > +\n> Same here:\n> size_t git_config_size_t(const char *name, const char *value) {\n> \tsize_t ret;\n> \tif (!git_parse_size_t(value, &ret))\n> \t\tdie_bad_number(name, value);\n> \treturn ret;\n> }\n\nIn 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.\n"}]}