{"thread":{"id":"45598","subject":"[PATCH v4] http.postbuffer: allow full range of ssize_t values","startedAt":"2017-04-03T17:30:36Z","lastAt":"2017-04-03T23:49:00Z","messageCount":4,"participants":["David Turner","Torsten Bögershausen","Ramsay Jones"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"316099","messageId":"20170403173015.15327-1-dturner@twosigma.com","threadId":"45598","inReplyTo":null,"subject":"[PATCH v4] http.postbuffer: allow full range of ssize_t values","fromName":"David Turner","fromEmail":"dturner@twosigma.com","sentAt":"2017-04-03T17:30:15Z","receivedAt":"2017-04-03T17:30:36Z","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\nThis means that we need to use CURLOPT_POSTFIELDSIZE_LARGE to set the\nbuffer size.\n\nSigned-off-by: David Turner <dturner@twosigma.com>\n---\n cache.h       |  1 +\n config.c      | 17 +++++++++++++++++\n http.c        |  4 ++--\n http.h        |  2 +-\n remote-curl.c |  6 +++---\n 5 files changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex fbdf7a815a..5e6747dbb4 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 ssize_t git_config_ssize_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..de5b155a4e 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 int git_parse_ssize_t(const char *value, ssize_t *ret)\n+{\n+\tssize_t tmp;\n+\tif (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_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+ssize_t git_config_ssize_t(const char *name, const char *value)\n+{\n+\tssize_t ret;\n+\tif (!git_parse_ssize_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..22f8167ba2 100644\n--- a/http.c\n+++ b/http.c\n@@ -19,7 +19,7 @@ long int git_curl_ipresolve;\n #endif\n int active_requests;\n int http_is_verbose;\n-size_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n+ssize_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n \n #if LIBCURL_VERSION_NUM >= 0x070a06\n #define LIBCURL_CAN_HANDLE_AUTH_ANY\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_ssize_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;\ndiff --git a/http.h b/http.h\nindex 02bccb7b0c..f7bd3b26b0 100644\n--- a/http.h\n+++ b/http.h\n@@ -111,7 +111,7 @@ extern struct curl_slist *http_copy_default_headers(void);\n extern long int git_curl_ipresolve;\n extern int active_requests;\n extern int http_is_verbose;\n-extern size_t http_post_buffer;\n+extern ssize_t http_post_buffer;\n extern struct credential http_auth;\n \n extern char curl_errorstr[CURL_ERROR_SIZE];\ndiff --git a/remote-curl.c b/remote-curl.c\nindex e953d06f66..69b4d71e4c 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -614,7 +614,7 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t * and we just need to send it.\n \t\t */\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, gzip_size);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) gzip_size);\n \n \t} else if (use_gzip && 1024 < rpc->len) {\n \t\t/* The client backend isn't giving us compressed data so\n@@ -645,7 +645,7 @@ static int post_rpc(struct rpc_state *rpc)\n \n \t\theaders = curl_slist_append(headers, \"Content-Encoding: gzip\");\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, gzip_size);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) gzip_size);\n \n \t\tif (options.verbosity > 1) {\n \t\t\tfprintf(stderr, \"POST %s (gzip %lu to %lu bytes)\\n\",\n@@ -658,7 +658,7 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t * more normal Content-Length approach.\n \t\t */\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, rpc->buf);\n-\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, rpc->len);\n+\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) rpc->len);\n \t\tif (options.verbosity > 1) {\n \t\t\tfprintf(stderr, \"POST %s (%lu bytes)\\n\",\n \t\t\t\trpc->service_name, (unsigned long)rpc->len);\n-- \n2.11.GIT\n\n"},{"id":"316108","messageId":"1cc11732-cf47-8d07-cb57-1e962ba1613f@web.de","threadId":"45598","inReplyTo":"20170403173015.15327-1-dturner@twosigma.com","subject":"Re: [PATCH v4] http.postbuffer: allow full range of ssize_t values","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2017-04-03T19:11:00Z","receivedAt":"2017-04-03T19:11:23Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 03.04.17 19:30, 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> This means that we need to use CURLOPT_POSTFIELDSIZE_LARGE to set the\n> buffer size.\n> \n> Signed-off-by: David Turner <dturner@twosigma.com>\n> ---\n>  cache.h       |  1 +\n>  config.c      | 17 +++++++++++++++++\n>  http.c        |  4 ++--\n>  http.h        |  2 +-\n>  remote-curl.c |  6 +++---\n>  5 files changed, 24 insertions(+), 6 deletions(-)\n> \n> diff --git a/cache.h b/cache.h\n> index fbdf7a815a..5e6747dbb4 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 ssize_t git_config_ssize_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..de5b155a4e 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 int git_parse_ssize_t(const char *value, ssize_t *ret)\n> +{\n> +\tssize_t tmp;\n> +\tif (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_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> +ssize_t git_config_ssize_t(const char *name, const char *value)\n> +{\n> +\tssize_t ret;\n> +\tif (!git_parse_ssize_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)\n> diff --git a/http.c b/http.c\n> index 96d84bbed3..22f8167ba2 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -19,7 +19,7 @@ long int git_curl_ipresolve;\n>  #endif\n>  int active_requests;\n>  int http_is_verbose;\n> -size_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n> +ssize_t http_post_buffer = 16 * LARGE_PACKET_MAX;\n>  \n>  #if LIBCURL_VERSION_NUM >= 0x070a06\n>  #define LIBCURL_CAN_HANDLE_AUTH_ANY\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_ssize_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> diff --git a/http.h b/http.h\n> index 02bccb7b0c..f7bd3b26b0 100644\n> --- a/http.h\n> +++ b/http.h\n> @@ -111,7 +111,7 @@ extern struct curl_slist *http_copy_default_headers(void);\n>  extern long int git_curl_ipresolve;\n>  extern int active_requests;\n>  extern int http_is_verbose;\n> -extern size_t http_post_buffer;\n> +extern ssize_t http_post_buffer;\n>  extern struct credential http_auth;\n>  \n>  extern char curl_errorstr[CURL_ERROR_SIZE];\n> diff --git a/remote-curl.c b/remote-curl.c\n> index e953d06f66..69b4d71e4c 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -614,7 +614,7 @@ static int post_rpc(struct rpc_state *rpc)\n>  \t\t * and we just need to send it.\n>  \t\t */\n>  \t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);\n> -\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, gzip_size);\n> +\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) gzip_size);\n\nIs this cast safe, or can it silently truncate ?\nOr is it more save to die() in this case, similar to ehat we do in git-compat-util.h?\n\nstatic inline size_t xsize_t(off_t len)\n{\n\tif (len > (size_t) len)\n\t\tdie(\"Cannot handle files this big\");\n\treturn (size_t)len;\n}\n\n\n \n>  \n>  \t} else if (use_gzip && 1024 < rpc->len) {\n>  \t\t/* The client backend isn't giving us compressed data so\n> @@ -645,7 +645,7 @@ static int post_rpc(struct rpc_state *rpc)\n>  \n>  \t\theaders = curl_slist_append(headers, \"Content-Encoding: gzip\");\n>  \t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, gzip_body);\n> -\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, gzip_size);\n> +\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) gzip_size);\n>  \n>  \t\tif (options.verbosity > 1) {\n>  \t\t\tfprintf(stderr, \"POST %s (gzip %lu to %lu bytes)\\n\",\n> @@ -658,7 +658,7 @@ static int post_rpc(struct rpc_state *rpc)\n>  \t\t * more normal Content-Length approach.\n>  \t\t */\n>  \t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, rpc->buf);\n> -\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, rpc->len);\n> +\t\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) rpc->len);\n>  \t\tif (options.verbosity > 1) {\n>  \t\t\tfprintf(stderr, \"POST %s (%lu bytes)\\n\",\n>  \t\t\t\trpc->service_name, (unsigned long)rpc->len);\n> \n\n"},{"id":"316115","messageId":"233d3799-2d24-1d7e-c97f-bc41abb1411a@ramsayjones.plus.com","threadId":"45598","inReplyTo":"20170403173015.15327-1-dturner@twosigma.com","subject":"Re: [PATCH v4] http.postbuffer: allow full range of ssize_t values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-04-03T22:24:03Z","receivedAt":"2017-04-03T22:24:11Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 03/04/17 18:30, 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> This means that we need to use CURLOPT_POSTFIELDSIZE_LARGE to set the\n> buffer size.\n> \n> Signed-off-by: David Turner <dturner@twosigma.com>\n> ---\n>  cache.h       |  1 +\n>  config.c      | 17 +++++++++++++++++\n>  http.c        |  4 ++--\n>  http.h        |  2 +-\n>  remote-curl.c |  6 +++---\n>  5 files changed, 24 insertions(+), 6 deletions(-)\n> \n> diff --git a/cache.h b/cache.h\n> index fbdf7a815a..5e6747dbb4 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 ssize_t git_config_ssize_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..de5b155a4e 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 int git_parse_ssize_t(const char *value, ssize_t *ret)\n> +{\n> +\tssize_t tmp;\n> +\tif (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_t)))\n> +\t\treturn 0;\n> +\t*ret = tmp;\n> +\treturn 1;\n> +}\n> +\n\nThe previous version of this patch causes gcc to complain on\na Linux 32bit build, like so:\n\n    CC config.o\nconfig.c: In function ‘git_parse_ssize_t’:\nconfig.c:840:31: warning: passing argument 2 of ‘git_parse_signed’ from incompatible pointer type [-Wincompatible-pointer-types]\n  if (!git_parse_signed(value, &tmp, maximum_signed_value_of_type(ssize_t)))\n                               ^\nconfig.c:753:12: note: expected ‘intmax_t * {aka long long int *}’ but argument is of type ‘ssize_t * {aka int *}’\n static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)\n            ^\n\n... and I don't think this version would be any different (though I\nhaven't tried).\n\nATB,\nRamsay Jones\n\n"},{"id":"316122","messageId":"fab6b6b8380a410fad0edb4d30b697b6@exmbdft7.ad.twosigma.com","threadId":"45598","inReplyTo":"233d3799-2d24-1d7e-c97f-bc41abb1411a@ramsayjones.plus.com","subject":"RE: [PATCH v4] http.postbuffer: allow full range of ssize_t values","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2017-04-03T23:48:51Z","receivedAt":"2017-04-03T23:49:00Z","isPatch":true,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"\n\n> -----Original Message-----\n> From: Ramsay Jones [mailto:ramsay@ramsayjones.plus.com]\n> Sent: Monday, April 3, 2017 6:24 PM\n> To: David Turner <David.Turner@twosigma.com>; git@vger.kernel.org\n> Subject: Re: [PATCH v4] http.postbuffer: allow full range of ssize_t values\n> \n> \n> \n> On 03/04/17 18:30, 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> > This means that we need to use CURLOPT_POSTFIELDSIZE_LARGE to set\n> the\n> > buffer size.\n> >\n> > Signed-off-by: David Turner <dturner@twosigma.com>\n> > ---\n> >  cache.h       |  1 +\n> >  config.c      | 17 +++++++++++++++++\n> >  http.c        |  4 ++--\n> >  http.h        |  2 +-\n> >  remote-curl.c |  6 +++---\n> >  5 files changed, 24 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/cache.h b/cache.h\n> > index fbdf7a815a..5e6747dbb4 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 ssize_t git_config_ssize_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..de5b155a4e 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 int git_parse_ssize_t(const char *value, ssize_t *ret) {\n> > +\tssize_t tmp;\n> > +\tif (!git_parse_signed(value, &tmp,\n> maximum_signed_value_of_type(ssize_t)))\n> > +\t\treturn 0;\n> > +\t*ret = tmp;\n> > +\treturn 1;\n> > +}\n> > +\n> \n> The previous version of this patch causes gcc to complain on a Linux 32bit\n> build, like so:\n> \n>     CC config.o\n> config.c: In function ‘git_parse_ssize_t’:\n> config.c:840:31: warning: passing argument 2 of ‘git_parse_signed’ from\n> incompatible pointer type [-Wincompatible-pointer-types]\n>   if (!git_parse_signed(value, &tmp,\n> maximum_signed_value_of_type(ssize_t)))\n>                                ^\n> config.c:753:12: note: expected ‘intmax_t * {aka long long int *}’ but\n> argument is of type ‘ssize_t * {aka int *}’\n>  static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)\n>             ^\n> \n> ... and I don't think this version would be any different (though I haven't\n> tried).\n\nSo I guess that means tmp needs to be an intmax_t.\n"}]}