{"thread":{"id":"49546","subject":"[PATCH] zlib.c: use size_t for size","startedAt":"2018-10-12T07:07:32Z","lastAt":"2018-10-15T05:55:22Z","messageCount":19,"participants":["Junio C Hamano","Johannes Schindelin","tboegi@web.de","SZEDER Gábor","Ramsay Jones","Jeff King","Torsten Bögershausen","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"360277","messageId":"xmqqsh1bbq36.fsf@gitster-ct.c.googlers.com","threadId":"49546","inReplyTo":null,"subject":"[PATCH] zlib.c: use size_t for size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-12T07:07:25Z","receivedAt":"2018-10-12T07:07:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Martin Koegler <martin.koegler@chello.at>\nDate: Thu, 10 Aug 2017 20:13:08 +0200\n\nSigned-off-by: Martin Koegler <martin.koegler@chello.at>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I made minimal adjustments to make the change apply to today's\n   codebase.  I still find some choices and mixing of off_t and\n   size_t done by the patch a bit iffy, and some arith may need to\n   become st_addX().  Extra set of eyes are very much appreciated.\n\n builtin/pack-objects.c | 10 +++++-----\n cache.h                | 10 +++++-----\n pack-check.c           |  6 +++---\n pack.h                 |  2 +-\n packfile.h             |  2 +-\n wrapper.c              |  8 ++++----\n zlib.c                 |  8 ++++----\n 7 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex e6316d294d..b9ca04eb8a 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,\n \t\tstruct packed_git *p,\n \t\tstruct pack_window **w_curs,\n \t\toff_t offset,\n-\t\toff_t len)\n+\t\tsize_t len)\n {\n \tunsigned char *in;\n-\tunsigned long avail;\n+\tsize_t avail;\n \n \twhile (len) {\n \t\tin = use_pack(p, w_curs, offset, &avail);\n \t\tif (avail > len)\n-\t\t\tavail = (unsigned long)len;\n+\t\tavail = len;\n \t\thashwrite(f, in, avail);\n \t\toffset += avail;\n \t\tlen -= avail;\n@@ -1478,8 +1478,8 @@ static void check_object(struct object_entry *entry)\n \t\tstruct pack_window *w_curs = NULL;\n \t\tconst unsigned char *base_ref = NULL;\n \t\tstruct object_entry *base_entry;\n-\t\tunsigned long used, used_0;\n-\t\tunsigned long avail;\n+\t\tsize_t used, used_0;\n+\t\tsize_t avail;\n \t\toff_t ofs;\n \t\tunsigned char *buf, c;\n \t\tenum object_type type;\ndiff --git a/cache.h b/cache.h\nindex d508f3d4f8..fce53fe620 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -20,10 +20,10 @@\n #include <zlib.h>\n typedef struct git_zstream {\n \tz_stream z;\n-\tunsigned long avail_in;\n-\tunsigned long avail_out;\n-\tunsigned long total_in;\n-\tunsigned long total_out;\n+\tsize_t avail_in;\n+\tsize_t avail_out;\n+\tsize_t total_in;\n+\tsize_t total_out;\n \tunsigned char *next_in;\n \tunsigned char *next_out;\n } git_zstream;\n@@ -40,7 +40,7 @@ void git_deflate_end(git_zstream *);\n int git_deflate_abort(git_zstream *);\n int git_deflate_end_gently(git_zstream *);\n int git_deflate(git_zstream *, int flush);\n-unsigned long git_deflate_bound(git_zstream *, unsigned long);\n+size_t git_deflate_bound(git_zstream *, size_t);\n \n /* The length in bytes and in hex digits of an object name (SHA-1 value). */\n #define GIT_SHA1_RAWSZ 20\ndiff --git a/pack-check.c b/pack-check.c\nindex fa5f0ff8fa..575e3e7125 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -27,13 +27,13 @@ static int compare_entries(const void *e1, const void *e2)\n }\n \n int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n-\t\t   off_t offset, off_t len, unsigned int nr)\n+\t\t   off_t offset, size_t len, unsigned int nr)\n {\n \tconst uint32_t *index_crc;\n \tuint32_t data_crc = crc32(0, NULL, 0);\n \n \tdo {\n-\t\tunsigned long avail;\n+\t\tsize_t avail;\n \t\tvoid *data = use_pack(p, w_curs, offset, &avail);\n \t\tif (avail > len)\n \t\t\tavail = len;\n@@ -68,7 +68,7 @@ static int verify_packfile(struct packed_git *p,\n \n \tthe_hash_algo->init_fn(&ctx);\n \tdo {\n-\t\tunsigned long remaining;\n+\t\tsize_t remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n \t\toffset += remaining;\n \t\tif (!pack_sig_ofs)\ndiff --git a/pack.h b/pack.h\nindex 34a9d458b4..1c9fecf929 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -78,7 +78,7 @@ struct progress;\n typedef int (*verify_fn)(const struct object_id *, enum object_type, unsigned long, void*, int*);\n \n extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const unsigned char *sha1);\n-extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n+extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, size_t len, unsigned int nr);\n extern int verify_pack_index(struct packed_git *);\n extern int verify_pack(struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n extern off_t write_pack_header(struct hashfile *f, uint32_t);\ndiff --git a/packfile.h b/packfile.h\nindex 442625723d..e2daf63426 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -78,7 +78,7 @@ extern void close_pack_index(struct packed_git *);\n \n extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);\n \n-extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);\n+extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, size_t *);\n extern void close_pack_windows(struct packed_git *);\n extern void close_pack(struct packed_git *);\n extern void close_all_packs(struct raw_object_store *o);\ndiff --git a/wrapper.c b/wrapper.c\nindex e4fa9d84cd..1a510bd6fc 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -67,11 +67,11 @@ static void *do_xmalloc(size_t size, int gentle)\n \t\t\tret = malloc(1);\n \t\tif (!ret) {\n \t\t\tif (!gentle)\n-\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t\t    (unsigned long)size);\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n+\t\t\t\t    (uintmax_t)size);\n \t\t\telse {\n-\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t\t      (unsigned long)size);\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n+\t\t\t\t      (uintmax_t)size);\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t}\ndiff --git a/zlib.c b/zlib.c\nindex d594cba3fc..197a1acc7b 100644\n--- a/zlib.c\n+++ b/zlib.c\n@@ -29,7 +29,7 @@ static const char *zerr_to_string(int status)\n  */\n /* #define ZLIB_BUF_MAX ((uInt)-1) */\n #define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */\n-static inline uInt zlib_buf_cap(unsigned long len)\n+static inline uInt zlib_buf_cap(size_t len)\n {\n \treturn (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;\n }\n@@ -46,8 +46,8 @@ static void zlib_pre_call(git_zstream *s)\n \n static void zlib_post_call(git_zstream *s)\n {\n-\tunsigned long bytes_consumed;\n-\tunsigned long bytes_produced;\n+\tsize_t bytes_consumed;\n+\tsize_t bytes_produced;\n \n \tbytes_consumed = s->z.next_in - s->next_in;\n \tbytes_produced = s->z.next_out - s->next_out;\n@@ -150,7 +150,7 @@ int git_inflate(git_zstream *strm, int flush)\n #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)\n #endif\n \n-unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)\n+size_t git_deflate_bound(git_zstream *strm, size_t size)\n {\n \treturn deflateBound(&strm->z, size);\n }\n-- \n2.19.1-328-g5a0cc8aca7\n\n"},{"id":"360292","messageId":"nycvar.QRO.7.76.6.1810121150050.45@tvgsbejvaqbjf.bet","threadId":"49546","inReplyTo":"xmqqsh1bbq36.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-12T09:54:31Z","receivedAt":"2018-10-12T09:54:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 12 Oct 2018, Junio C Hamano wrote:\n\n> From: Martin Koegler <martin.koegler@chello.at>\n> Date: Thu, 10 Aug 2017 20:13:08 +0200\n> \n> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * I made minimal adjustments to make the change apply to today's\n>    codebase.  I still find some choices and mixing of off_t and\n>    size_t done by the patch a bit iffy, and some arith may need to\n>    become st_addX().  Extra set of eyes are very much appreciated.\n> \n>  builtin/pack-objects.c | 10 +++++-----\n>  cache.h                | 10 +++++-----\n>  pack-check.c           |  6 +++---\n>  pack.h                 |  2 +-\n>  packfile.h             |  2 +-\n>  wrapper.c              |  8 ++++----\n>  zlib.c                 |  8 ++++----\n>  7 files changed, 23 insertions(+), 23 deletions(-)\n> \n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index e6316d294d..b9ca04eb8a 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,\n>  \t\tstruct packed_git *p,\n>  \t\tstruct pack_window **w_curs,\n>  \t\toff_t offset,\n> -\t\toff_t len)\n> +\t\tsize_t len)\n>  {\n>  \tunsigned char *in;\n> -\tunsigned long avail;\n> +\tsize_t avail;\n>  \n>  \twhile (len) {\n>  \t\tin = use_pack(p, w_curs, offset, &avail);\n>  \t\tif (avail > len)\n\nDo we have to be careful to handle sizeof(off_t) != sizeof(size_t) here? I\nguess we would receive a compiler warning about different sizes in that\ncase, but it makes me worry.\n\nNot that it matters, as we currently have an *even worse* situation where\nwe *know* that `unsigned long` is 4 bytes on 64-bit Windows, and `off_t`\n(the version we use, at least) is 8 bytes.\n\n> -\t\t\tavail = (unsigned long)len;\n> +\t\tavail = len;\n\nIndentation change?\n\nApart from the consideration about different sizes of the data types\n(which could maybe addressed via a comment in the commit message, after\nthinking deeply about it), I think this patch is good to go.\n\nCiao,\nDscho\n\n>  \t\thashwrite(f, in, avail);\n>  \t\toffset += avail;\n>  \t\tlen -= avail;\n> @@ -1478,8 +1478,8 @@ static void check_object(struct object_entry *entry)\n>  \t\tstruct pack_window *w_curs = NULL;\n>  \t\tconst unsigned char *base_ref = NULL;\n>  \t\tstruct object_entry *base_entry;\n> -\t\tunsigned long used, used_0;\n> -\t\tunsigned long avail;\n> +\t\tsize_t used, used_0;\n> +\t\tsize_t avail;\n>  \t\toff_t ofs;\n>  \t\tunsigned char *buf, c;\n>  \t\tenum object_type type;\n> diff --git a/cache.h b/cache.h\n> index d508f3d4f8..fce53fe620 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -20,10 +20,10 @@\n>  #include <zlib.h>\n>  typedef struct git_zstream {\n>  \tz_stream z;\n> -\tunsigned long avail_in;\n> -\tunsigned long avail_out;\n> -\tunsigned long total_in;\n> -\tunsigned long total_out;\n> +\tsize_t avail_in;\n> +\tsize_t avail_out;\n> +\tsize_t total_in;\n> +\tsize_t total_out;\n>  \tunsigned char *next_in;\n>  \tunsigned char *next_out;\n>  } git_zstream;\n> @@ -40,7 +40,7 @@ void git_deflate_end(git_zstream *);\n>  int git_deflate_abort(git_zstream *);\n>  int git_deflate_end_gently(git_zstream *);\n>  int git_deflate(git_zstream *, int flush);\n> -unsigned long git_deflate_bound(git_zstream *, unsigned long);\n> +size_t git_deflate_bound(git_zstream *, size_t);\n>  \n>  /* The length in bytes and in hex digits of an object name (SHA-1 value). */\n>  #define GIT_SHA1_RAWSZ 20\n> diff --git a/pack-check.c b/pack-check.c\n> index fa5f0ff8fa..575e3e7125 100644\n> --- a/pack-check.c\n> +++ b/pack-check.c\n> @@ -27,13 +27,13 @@ static int compare_entries(const void *e1, const void *e2)\n>  }\n>  \n>  int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n> -\t\t   off_t offset, off_t len, unsigned int nr)\n> +\t\t   off_t offset, size_t len, unsigned int nr)\n>  {\n>  \tconst uint32_t *index_crc;\n>  \tuint32_t data_crc = crc32(0, NULL, 0);\n>  \n>  \tdo {\n> -\t\tunsigned long avail;\n> +\t\tsize_t avail;\n>  \t\tvoid *data = use_pack(p, w_curs, offset, &avail);\n>  \t\tif (avail > len)\n>  \t\t\tavail = len;\n> @@ -68,7 +68,7 @@ static int verify_packfile(struct packed_git *p,\n>  \n>  \tthe_hash_algo->init_fn(&ctx);\n>  \tdo {\n> -\t\tunsigned long remaining;\n> +\t\tsize_t remaining;\n>  \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n>  \t\toffset += remaining;\n>  \t\tif (!pack_sig_ofs)\n> diff --git a/pack.h b/pack.h\n> index 34a9d458b4..1c9fecf929 100644\n> --- a/pack.h\n> +++ b/pack.h\n> @@ -78,7 +78,7 @@ struct progress;\n>  typedef int (*verify_fn)(const struct object_id *, enum object_type, unsigned long, void*, int*);\n>  \n>  extern const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const unsigned char *sha1);\n> -extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n> +extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, size_t len, unsigned int nr);\n>  extern int verify_pack_index(struct packed_git *);\n>  extern int verify_pack(struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n>  extern off_t write_pack_header(struct hashfile *f, uint32_t);\n> diff --git a/packfile.h b/packfile.h\n> index 442625723d..e2daf63426 100644\n> --- a/packfile.h\n> +++ b/packfile.h\n> @@ -78,7 +78,7 @@ extern void close_pack_index(struct packed_git *);\n>  \n>  extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);\n>  \n> -extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);\n> +extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, size_t *);\n>  extern void close_pack_windows(struct packed_git *);\n>  extern void close_pack(struct packed_git *);\n>  extern void close_all_packs(struct raw_object_store *o);\n> diff --git a/wrapper.c b/wrapper.c\n> index e4fa9d84cd..1a510bd6fc 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -67,11 +67,11 @@ static void *do_xmalloc(size_t size, int gentle)\n>  \t\t\tret = malloc(1);\n>  \t\tif (!ret) {\n>  \t\t\tif (!gentle)\n> -\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> -\t\t\t\t    (unsigned long)size);\n> +\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n> +\t\t\t\t    (uintmax_t)size);\n>  \t\t\telse {\n> -\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> -\t\t\t\t      (unsigned long)size);\n> +\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n> +\t\t\t\t      (uintmax_t)size);\n>  \t\t\t\treturn NULL;\n>  \t\t\t}\n>  \t\t}\n> diff --git a/zlib.c b/zlib.c\n> index d594cba3fc..197a1acc7b 100644\n> --- a/zlib.c\n> +++ b/zlib.c\n> @@ -29,7 +29,7 @@ static const char *zerr_to_string(int status)\n>   */\n>  /* #define ZLIB_BUF_MAX ((uInt)-1) */\n>  #define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */\n> -static inline uInt zlib_buf_cap(unsigned long len)\n> +static inline uInt zlib_buf_cap(size_t len)\n>  {\n>  \treturn (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;\n>  }\n> @@ -46,8 +46,8 @@ static void zlib_pre_call(git_zstream *s)\n>  \n>  static void zlib_post_call(git_zstream *s)\n>  {\n> -\tunsigned long bytes_consumed;\n> -\tunsigned long bytes_produced;\n> +\tsize_t bytes_consumed;\n> +\tsize_t bytes_produced;\n>  \n>  \tbytes_consumed = s->z.next_in - s->next_in;\n>  \tbytes_produced = s->z.next_out - s->next_out;\n> @@ -150,7 +150,7 @@ int git_inflate(git_zstream *strm, int flush)\n>  #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)\n>  #endif\n>  \n> -unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)\n> +size_t git_deflate_bound(git_zstream *strm, size_t size)\n>  {\n>  \treturn deflateBound(&strm->z, size);\n>  }\n> -- \n> 2.19.1-328-g5a0cc8aca7\n> \n> \n"},{"id":"360310","messageId":"xmqqzhvj9ssa.fsf@gitster-ct.c.googlers.com","threadId":"49546","inReplyTo":"nycvar.QRO.7.76.6.1810121150050.45@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-12T13:52:05Z","receivedAt":"2018-10-12T13:52:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Junio,\n>\n> On Fri, 12 Oct 2018, Junio C Hamano wrote:\n>\n>> From: Martin Koegler <martin.koegler@chello.at>\n>> Date: Thu, 10 Aug 2017 20:13:08 +0200\n>> \n>> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>> ---\n>> \n>>  * I made minimal adjustments to make the change apply to today's\n>>    codebase.  I still find some choices and mixing of off_t and\n>>    size_t done by the patch a bit iffy, and some arith may need to\n>>    become st_addX().  Extra set of eyes are very much appreciated.\n>> \n>>  builtin/pack-objects.c | 10 +++++-----\n>>  cache.h                | 10 +++++-----\n>>  pack-check.c           |  6 +++---\n>>  pack.h                 |  2 +-\n>>  packfile.h             |  2 +-\n>>  wrapper.c              |  8 ++++----\n>>  zlib.c                 |  8 ++++----\n>>  7 files changed, 23 insertions(+), 23 deletions(-)\n>> \n>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n>> index e6316d294d..b9ca04eb8a 100644\n>> --- a/builtin/pack-objects.c\n>> +++ b/builtin/pack-objects.c\n>> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,\n>>  \t\tstruct packed_git *p,\n>>  \t\tstruct pack_window **w_curs,\n>>  \t\toff_t offset,\n>> -\t\toff_t len)\n>> +\t\tsize_t len)\n>>  {\n>>  \tunsigned char *in;\n>> -\tunsigned long avail;\n>> +\tsize_t avail;\n>>  \n>>  \twhile (len) {\n>>  \t\tin = use_pack(p, w_curs, offset, &avail);\n>>  \t\tif (avail > len)\n>\n> Do we have to be careful to handle sizeof(off_t) != sizeof(size_t) here? I\n> guess we would receive a compiler warning about different sizes in that\n> case, but it makes me worry.\n\nYup, you just said exactly the same thing as I already said in the\npart you quoted.  I still find the mixed use of off_t and size_t in\nthis patch iffy, and that was the secondary reason why the patch was\nkept in the stalled state for so long.  The primary reason was that\nnobody tried to dust it off and reignite the topic so far---which I\nam trying to correct, but as I said, this is just minimally adjusted\nto today's codebase, without any attempt to improve relative to the\noriginal patch.\n\nI think we are in agreement in that this is not making things worse,\nas we are already in the mixed arith territory before this patch.\n\n"},{"id":"360322","messageId":"nycvar.QRO.7.76.6.1810121733560.561@tvgsbejvaqbjf.bet","threadId":"49546","inReplyTo":"xmqqzhvj9ssa.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-10-12T15:34:26Z","receivedAt":"2018-10-12T15:34:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 12 Oct 2018, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Hi Junio,\n> >\n> > On Fri, 12 Oct 2018, Junio C Hamano wrote:\n> >\n> >> From: Martin Koegler <martin.koegler@chello.at>\n> >> Date: Thu, 10 Aug 2017 20:13:08 +0200\n> >> \n> >> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n> >> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> >> ---\n> >> \n> >>  * I made minimal adjustments to make the change apply to today's\n> >>    codebase.  I still find some choices and mixing of off_t and\n> >>    size_t done by the patch a bit iffy, and some arith may need to\n> >>    become st_addX().  Extra set of eyes are very much appreciated.\n> >> \n> >>  builtin/pack-objects.c | 10 +++++-----\n> >>  cache.h                | 10 +++++-----\n> >>  pack-check.c           |  6 +++---\n> >>  pack.h                 |  2 +-\n> >>  packfile.h             |  2 +-\n> >>  wrapper.c              |  8 ++++----\n> >>  zlib.c                 |  8 ++++----\n> >>  7 files changed, 23 insertions(+), 23 deletions(-)\n> >> \n> >> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> >> index e6316d294d..b9ca04eb8a 100644\n> >> --- a/builtin/pack-objects.c\n> >> +++ b/builtin/pack-objects.c\n> >> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,\n> >>  \t\tstruct packed_git *p,\n> >>  \t\tstruct pack_window **w_curs,\n> >>  \t\toff_t offset,\n> >> -\t\toff_t len)\n> >> +\t\tsize_t len)\n> >>  {\n> >>  \tunsigned char *in;\n> >> -\tunsigned long avail;\n> >> +\tsize_t avail;\n> >>  \n> >>  \twhile (len) {\n> >>  \t\tin = use_pack(p, w_curs, offset, &avail);\n> >>  \t\tif (avail > len)\n> >\n> > Do we have to be careful to handle sizeof(off_t) != sizeof(size_t) here? I\n> > guess we would receive a compiler warning about different sizes in that\n> > case, but it makes me worry.\n> \n> Yup, you just said exactly the same thing as I already said in the\n> part you quoted.  I still find the mixed use of off_t and size_t in\n> this patch iffy, and that was the secondary reason why the patch was\n> kept in the stalled state for so long.  The primary reason was that\n> nobody tried to dust it off and reignite the topic so far---which I\n> am trying to correct, but as I said, this is just minimally adjusted\n> to today's codebase, without any attempt to improve relative to the\n> original patch.\n> \n> I think we are in agreement in that this is not making things worse,\n> as we are already in the mixed arith territory before this patch.\n\nI will *try* to find the time to audit this some more, then. Maybe next\nweek, maybe the one afterwards... ;-)\n\nCiao,\nDscho\n\n> \n> \n"},{"id":"360349","messageId":"20181012204229.11890-1-tboegi@web.de","threadId":"49546","inReplyTo":"xmqqsh1bbq36.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v2 1/1] zlib.c: use size_t for size","fromName":"","fromEmail":"tboegi@web.de","sentAt":"2018-10-12T20:42:29Z","receivedAt":"2018-10-12T20:42:42Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"From: Martin Koegler <martin.koegler@chello.at>\n\nSigned-off-by: Martin Koegler <martin.koegler@chello.at>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n\nAfter doing a review, I decided to send the result as a patch.\nIn general, the changes from off_t to size_t seem to be not really\nmotivated.\nBut if they are, they could and should go into an own patch.\nFor the moment, change only \"unsigned long\" into size_t, thats all\n\n builtin/pack-objects.c |  8 ++++----\n cache.h                | 10 +++++-----\n pack-check.c           |  4 ++--\n packfile.h             |  2 +-\n wrapper.c              |  8 ++++----\n zlib.c                 |  8 ++++----\n 6 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex e6316d294d..23c4cd8c77 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,\n \t\toff_t len)\n {\n \tunsigned char *in;\n-\tunsigned long avail;\n+\tsize_t avail;\n \n \twhile (len) {\n \t\tin = use_pack(p, w_curs, offset, &avail);\n \t\tif (avail > len)\n-\t\t\tavail = (unsigned long)len;\n+\t\t\tavail = xsize_t(len);\n \t\thashwrite(f, in, avail);\n \t\toffset += avail;\n \t\tlen -= avail;\n@@ -1478,8 +1478,8 @@ static void check_object(struct object_entry *entry)\n \t\tstruct pack_window *w_curs = NULL;\n \t\tconst unsigned char *base_ref = NULL;\n \t\tstruct object_entry *base_entry;\n-\t\tunsigned long used, used_0;\n-\t\tunsigned long avail;\n+\t\tsize_t used, used_0;\n+\t\tsize_t avail;\n \t\toff_t ofs;\n \t\tunsigned char *buf, c;\n \t\tenum object_type type;\ndiff --git a/cache.h b/cache.h\nindex d508f3d4f8..fce53fe620 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -20,10 +20,10 @@\n #include <zlib.h>\n typedef struct git_zstream {\n \tz_stream z;\n-\tunsigned long avail_in;\n-\tunsigned long avail_out;\n-\tunsigned long total_in;\n-\tunsigned long total_out;\n+\tsize_t avail_in;\n+\tsize_t avail_out;\n+\tsize_t total_in;\n+\tsize_t total_out;\n \tunsigned char *next_in;\n \tunsigned char *next_out;\n } git_zstream;\n@@ -40,7 +40,7 @@ void git_deflate_end(git_zstream *);\n int git_deflate_abort(git_zstream *);\n int git_deflate_end_gently(git_zstream *);\n int git_deflate(git_zstream *, int flush);\n-unsigned long git_deflate_bound(git_zstream *, unsigned long);\n+size_t git_deflate_bound(git_zstream *, size_t);\n \n /* The length in bytes and in hex digits of an object name (SHA-1 value). */\n #define GIT_SHA1_RAWSZ 20\ndiff --git a/pack-check.c b/pack-check.c\nindex fa5f0ff8fa..d1e7f554ae 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -33,7 +33,7 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n \tuint32_t data_crc = crc32(0, NULL, 0);\n \n \tdo {\n-\t\tunsigned long avail;\n+\t\tsize_t avail;\n \t\tvoid *data = use_pack(p, w_curs, offset, &avail);\n \t\tif (avail > len)\n \t\t\tavail = len;\n@@ -68,7 +68,7 @@ static int verify_packfile(struct packed_git *p,\n \n \tthe_hash_algo->init_fn(&ctx);\n \tdo {\n-\t\tunsigned long remaining;\n+\t\tsize_t remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n \t\toffset += remaining;\n \t\tif (!pack_sig_ofs)\ndiff --git a/packfile.h b/packfile.h\nindex 442625723d..e2daf63426 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -78,7 +78,7 @@ extern void close_pack_index(struct packed_git *);\n \n extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);\n \n-extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);\n+extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, size_t *);\n extern void close_pack_windows(struct packed_git *);\n extern void close_pack(struct packed_git *);\n extern void close_all_packs(struct raw_object_store *o);\ndiff --git a/wrapper.c b/wrapper.c\nindex e4fa9d84cd..1a510bd6fc 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -67,11 +67,11 @@ static void *do_xmalloc(size_t size, int gentle)\n \t\t\tret = malloc(1);\n \t\tif (!ret) {\n \t\t\tif (!gentle)\n-\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t\t    (unsigned long)size);\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n+\t\t\t\t    (uintmax_t)size);\n \t\t\telse {\n-\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t\t      (unsigned long)size);\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n+\t\t\t\t      (uintmax_t)size);\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t}\ndiff --git a/zlib.c b/zlib.c\nindex d594cba3fc..197a1acc7b 100644\n--- a/zlib.c\n+++ b/zlib.c\n@@ -29,7 +29,7 @@ static const char *zerr_to_string(int status)\n  */\n /* #define ZLIB_BUF_MAX ((uInt)-1) */\n #define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */\n-static inline uInt zlib_buf_cap(unsigned long len)\n+static inline uInt zlib_buf_cap(size_t len)\n {\n \treturn (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;\n }\n@@ -46,8 +46,8 @@ static void zlib_pre_call(git_zstream *s)\n \n static void zlib_post_call(git_zstream *s)\n {\n-\tunsigned long bytes_consumed;\n-\tunsigned long bytes_produced;\n+\tsize_t bytes_consumed;\n+\tsize_t bytes_produced;\n \n \tbytes_consumed = s->z.next_in - s->next_in;\n \tbytes_produced = s->z.next_out - s->next_out;\n@@ -150,7 +150,7 @@ int git_inflate(git_zstream *strm, int flush)\n #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)\n #endif\n \n-unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)\n+size_t git_deflate_bound(git_zstream *strm, size_t size)\n {\n \treturn deflateBound(&strm->z, size);\n }\n-- \n2.19.0.271.gfe8321ec05\n\n"},{"id":"360357","messageId":"20181012222234.GF19800@szeder.dev","threadId":"49546","inReplyTo":"20181012204229.11890-1-tboegi@web.de","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-10-12T22:22:34Z","receivedAt":"2018-10-12T22:22:40Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 12, 2018 at 10:42:29PM +0200, tboegi@web.de wrote:\n> From: Martin Koegler <martin.koegler@chello.at>\n> \n> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> ---\n> \n> After doing a review, I decided to send the result as a patch.\n> In general, the changes from off_t to size_t seem to be not really\n> motivated.\n> But if they are, they could and should go into an own patch.\n> For the moment, change only \"unsigned long\" into size_t, thats all\n\nNeither v1 nor v2 of this patch compiles on 32 bit Linux; see\n\n  https://travis-ci.org/git/git/jobs/440514375#L628\n\nThe fixup patch below makes it compile on 32 bit and the test suite\npasses as well, but I didn't thoroughly review the changes; I only\nwanted to get 'pu' build again.\n\n\n  --  >8 --\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 23c4cd8c77..89fe1c5d46 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1966,7 +1966,8 @@ unsigned long oe_get_size_slow(struct packing_data *pack,\n \tstruct pack_window *w_curs;\n \tunsigned char *buf;\n \tenum object_type type;\n-\tunsigned long used, avail, size;\n+\tunsigned long used, size;\n+\tsize_t avail;\n \n \tif (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {\n \t\tread_lock();\ndiff --git a/packfile.c b/packfile.c\nindex 841b36182f..9f50411ad3 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -579,7 +579,7 @@ static int in_window(struct pack_window *win, off_t offset)\n unsigned char *use_pack(struct packed_git *p,\n \t\tstruct pack_window **w_cursor,\n \t\toff_t offset,\n-\t\tunsigned long *left)\n+\t\tsize_t *left)\n {\n \tstruct pack_window *win = *w_cursor;\n \n@@ -1098,7 +1098,7 @@ int unpack_object_header(struct packed_git *p,\n \t\t\t unsigned long *sizep)\n {\n \tunsigned char *base;\n-\tunsigned long left;\n+\tsize_t left;\n \tunsigned long used;\n \tenum object_type type;\n \n"},{"id":"360361","messageId":"61d49c5f-826f-f28b-8eac-84a5abfab0ee@ramsayjones.plus.com","threadId":"49546","inReplyTo":"nycvar.QRO.7.76.6.1810121733560.561@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-12T23:23:05Z","receivedAt":"2018-10-12T23:23:12Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 12/10/18 16:34, Johannes Schindelin wrote:\n> Hi Junio,\n> \n> On Fri, 12 Oct 2018, Junio C Hamano wrote:\n> \n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>>\n>>> Hi Junio,\n>>>\n>>> On Fri, 12 Oct 2018, Junio C Hamano wrote:\n>>>\n>>>> From: Martin Koegler <martin.koegler@chello.at>\n>>>> Date: Thu, 10 Aug 2017 20:13:08 +0200\n>>>>\n>>>> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n>>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>>>> ---\n>>>>\n>>>>  * I made minimal adjustments to make the change apply to today's\n>>>>    codebase.  I still find some choices and mixing of off_t and\n>>>>    size_t done by the patch a bit iffy, and some arith may need to\n>>>>    become st_addX().  Extra set of eyes are very much appreciated.\n>>>>\n>>>>  builtin/pack-objects.c | 10 +++++-----\n>>>>  cache.h                | 10 +++++-----\n>>>>  pack-check.c           |  6 +++---\n>>>>  pack.h                 |  2 +-\n>>>>  packfile.h             |  2 +-\n>>>>  wrapper.c              |  8 ++++----\n>>>>  zlib.c                 |  8 ++++----\n>>>>  7 files changed, 23 insertions(+), 23 deletions(-)\n>>>>\n>>>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n>>>> index e6316d294d..b9ca04eb8a 100644\n>>>> --- a/builtin/pack-objects.c\n>>>> +++ b/builtin/pack-objects.c\n>>>> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,\n>>>>  \t\tstruct packed_git *p,\n>>>>  \t\tstruct pack_window **w_curs,\n>>>>  \t\toff_t offset,\n>>>> -\t\toff_t len)\n>>>> +\t\tsize_t len)\n>>>>  {\n>>>>  \tunsigned char *in;\n>>>> -\tunsigned long avail;\n>>>> +\tsize_t avail;\n>>>>  \n>>>>  \twhile (len) {\n>>>>  \t\tin = use_pack(p, w_curs, offset, &avail);\n>>>>  \t\tif (avail > len)\n>>>\n>>> Do we have to be careful to handle sizeof(off_t) != sizeof(size_t) here? I\n>>> guess we would receive a compiler warning about different sizes in that\n>>> case, but it makes me worry.\n>>\n>> Yup, you just said exactly the same thing as I already said in the\n>> part you quoted.  I still find the mixed use of off_t and size_t in\n>> this patch iffy, and that was the secondary reason why the patch was\n>> kept in the stalled state for so long.  The primary reason was that\n>> nobody tried to dust it off and reignite the topic so far---which I\n>> am trying to correct, but as I said, this is just minimally adjusted\n>> to today's codebase, without any attempt to improve relative to the\n>> original patch.\n>>\n>> I think we are in agreement in that this is not making things worse,\n>> as we are already in the mixed arith territory before this patch.\n> \n> I will *try* to find the time to audit this some more, then. Maybe next\n> week, maybe the one afterwards... ;-)\n\nThis fails to compile on 32-bit Linux. The patch given below is\nwhat I used to get git to build on 32-bit Linux (and it even\npasses the tests).\n\nI haven't even given the off_t -> size_t issue any thought, but I\nsuspect that will have to change as well. Anyway, I have no more\ntime tonight ...\n\nATB,\nRamsay Jones\n\n-- >8 --\nSubject: [PATCH] zlib: minimum fix-up on 32-bit linux\n\nSigned-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n---\n builtin/pack-objects.c |  3 ++-\n packfile.c             | 16 ++++++++--------\n packfile.h             |  2 +-\n 3 files changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 7e693071b..cc11a0426 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2002,7 +2002,8 @@ unsigned long oe_get_size_slow(struct packing_data *pack,\n \tstruct pack_window *w_curs;\n \tunsigned char *buf;\n \tenum object_type type;\n-\tunsigned long used, avail, size;\n+\tsize_t used, avail;\n+\tunsigned long size;\n \n \tif (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {\n \t\tread_lock();\ndiff --git a/packfile.c b/packfile.c\nindex f2850a00b..7571ac988 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -585,7 +585,7 @@ static int in_window(struct pack_window *win, off_t offset)\n unsigned char *use_pack(struct packed_git *p,\n \t\tstruct pack_window **w_cursor,\n \t\toff_t offset,\n-\t\tunsigned long *left)\n+\t\tsize_t *left)\n {\n \tstruct pack_window *win = *w_cursor;\n \n@@ -1034,19 +1034,19 @@ struct list_head *get_packed_git_mru(struct repository *r)\n \treturn &r->objects->packed_git_mru;\n }\n \n-unsigned long unpack_object_header_buffer(const unsigned char *buf,\n-\t\tunsigned long len, enum object_type *type, unsigned long *sizep)\n+size_t unpack_object_header_buffer(const unsigned char *buf,\n+\t\tsize_t len, enum object_type *type, unsigned long *sizep)\n {\n \tunsigned shift;\n-\tunsigned long size, c;\n-\tunsigned long used = 0;\n+\tsize_t size, c;\n+\tsize_t used = 0;\n \n \tc = buf[used++];\n \t*type = (c >> 4) & 7;\n \tsize = c & 15;\n \tshift = 4;\n \twhile (c & 0x80) {\n-\t\tif (len <= used || bitsizeof(long) <= shift) {\n+\t\tif (len <= used || bitsizeof(size_t) <= shift) {\n \t\t\terror(\"bad object header\");\n \t\t\tsize = used = 0;\n \t\t\tbreak;\n@@ -1104,8 +1104,8 @@ int unpack_object_header(struct packed_git *p,\n \t\t\t unsigned long *sizep)\n {\n \tunsigned char *base;\n-\tunsigned long left;\n-\tunsigned long used;\n+\tsize_t left;\n+\tsize_t used;\n \tenum object_type type;\n \n \t/* use_pack() assures us we have [base, base + 20) available\ndiff --git a/packfile.h b/packfile.h\nindex 1fb482424..e8bf11b62 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -132,7 +132,7 @@ extern off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *)\n \n extern int is_pack_valid(struct packed_git *);\n extern void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);\n-extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);\n+extern size_t unpack_object_header_buffer(const unsigned char *buf, size_t len, enum object_type *type, unsigned long *sizep);\n extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);\n extern int unpack_object_header(struct packed_git *, struct pack_window **, off_t *, unsigned long *);\n \n-- \n2.19.0\n\n"},{"id":"360376","messageId":"20181013023845.GA15595@sigill.intra.peff.net","threadId":"49546","inReplyTo":"xmqqsh1bbq36.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-13T02:38:45Z","receivedAt":"2018-10-13T02:38:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 12, 2018 at 04:07:25PM +0900, Junio C Hamano wrote:\n\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index e6316d294d..b9ca04eb8a 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -266,15 +266,15 @@ static void copy_pack_data(struct hashfile *f,\n>  \t\tstruct packed_git *p,\n>  \t\tstruct pack_window **w_curs,\n>  \t\toff_t offset,\n> -\t\toff_t len)\n> +\t\tsize_t len)\n>  {\n>  \tunsigned char *in;\n> -\tunsigned long avail;\n> +\tsize_t avail;\n\nI know there were a lot of comments about \"maybe this off_t switch is\nnot good\". Let me say something a bit stronger: I think this part of the\nchange is strictly worse.\n\ncopy_pack_data() looks like this right now:\n\n  static void copy_pack_data(struct hashfile *f,\n                  struct packed_git *p,\n                  struct pack_window **w_curs,\n                  off_t offset,\n                  off_t len)\n  {\n          unsigned char *in;\n          unsigned long avail;\n  \n          while (len) {\n                  in = use_pack(p, w_curs, offset, &avail);\n                  if (avail > len)\n                          avail = (unsigned long)len;\n                  hashwrite(f, in, avail);\n                  offset += avail;\n                  len -= avail;\n          }\n  }\n\nSo right now let's imagine that off_t is 64-bit, and \"unsigned long\" is\n32-bit (e.g., 32-bit system, or an IL32P64 model like Windows). We'll\nrepeatedly ask use_pack() for a window, and it will tell us how many\nbytes we have in \"avail\". So even as a 32-bit value, that just means\nwe'll process chunks smaller than 4GB, and this is correct (or at least\nthis part of it -- hold on). But we can still process the whole \"len\"\ngiven by the off_t eventually.\n\nBut by switching away from off_t in the function interface, we risk\ntruncation before we even enter the loop. Because of the switch to\nsize_t, it actually works on an IL32P64 system (because size_t is big\nthere), but it has introduced a bug on a true 32-bit system. If your\noff_t really is 64-bit (and it generally is because we #define\n_FILE_OFFSET_BITS), the function will truncate modulo 2^32.\n\nAnd nor will most compilers warn without -Wconversion. You can try it\nwith this on Linux:\n\n  #define _FILE_OFFSET_BITS 64\n  #include <unistd.h>\n  \n  void foo(size_t x);\n  void bar(off_t x);\n  \n  void bar(off_t x)\n  {\n  \n  \tfoo(x);\n  }\n\nThat compiles fine with \"gcc -c -m32 -Wall -Werror -Wextra\" for me.\nAdding \"-Wconversion\" catches it, but our code base is not close to\ncompiling with that warning enabled.\n\nSo I don't think this hunk is actually fixing any problems, and is\nactually introducing one.\n\nI do in general support moving to size_t over \"unsigned long\". Switching\navail to size_t makes sense here. It's just the off_t part that is\nfunny.\n\n-Peff\n"},{"id":"360377","messageId":"20181013024624.GB15595@sigill.intra.peff.net","threadId":"49546","inReplyTo":"20181013023845.GA15595@sigill.intra.peff.net","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-13T02:46:24Z","receivedAt":"2018-10-13T02:46:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 12, 2018 at 10:38:45PM -0400, Jeff King wrote:\n\n> So right now let's imagine that off_t is 64-bit, and \"unsigned long\" is\n> 32-bit (e.g., 32-bit system, or an IL32P64 model like Windows). We'll\n> repeatedly ask use_pack() for a window, and it will tell us how many\n> bytes we have in \"avail\". So even as a 32-bit value, that just means\n> we'll process chunks smaller than 4GB, and this is correct (or at least\n> this part of it -- hold on). But we can still process the whole \"len\"\n> given by the off_t eventually.\n\nSo this \"hold on\" was because I thought I had found another bug in\nuse_pack(), but I actually think it's OK.\n\nIn use_pack(), we do this:\n\n          if (left)\n                  *left = win->len - xsize_t(offset);\n\nwhere win->len is a size_t. Before this patch, \"left\" is a pointer to\nunsigned long. So that has the usual broken-on-Windows mismatch. This\npatch makes it a size_t, which is good.\n\nBut what's up with that xsize_t(offset)? We'll complain about truncation\n_before_ we do any offset subtraction. So at first glance, I thought\nthis meant we were broken for larger-than-4GB packs on 32-bit systems\nwhen trying to read past the 4GB mark.\n\nBut no, right before that we have this line:\n\n          offset -= win->offset;\n\nSo offset is in fact no longer its original meaning of \"offset into the\npackfile\", but is now an offset of the specific request into the window\nwe found.\n\nSo I think it's correct, but it sure confused me. I wonder if another\nvariable might help, like:\n\n  size_t offset_in_window;\n  ...\n\n  /*\n   * We know this difference will fit in a size_t, because our mmap\n   * window by * definition can be no larger than a size_t.\n   */\n  offset_in_window = xsize_t(offset - win->offset);\n  if (left)\n\t*left = win->len - offset_in_window;\n  return win->base + offset_in_window;\n\nI dunno. Maybe it is overkill.\n\n-Peff\n"},{"id":"360378","messageId":"20181013050057.GA6805@tor.lan","threadId":"49546","inReplyTo":"20181012222234.GF19800@szeder.dev","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-10-13T05:00:57Z","receivedAt":"2018-10-13T05:06:09Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"[]\n> Neither v1 nor v2 of this patch compiles on 32 bit Linux; see\n> \n>   https://travis-ci.org/git/git/jobs/440514375#L628\n> \n> The fixup patch below makes it compile on 32 bit and the test suite\n> passes as well, but I didn't thoroughly review the changes; I only\n> wanted to get 'pu' build again.\n> \n\nOh, yes, I didn't test under Linux 32 bit (and neither Windows)\nI will try to compose a proper v3 the next days.\n\n[snip the good stuff]\n"},{"id":"360387","messageId":"2e7b2f4b-92c3-a583-5985-38090cc78c64@kdbg.org","threadId":"49546","inReplyTo":"20181013024624.GB15595@sigill.intra.peff.net","subject":"Re: [PATCH] zlib.c: use size_t for size","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-10-13T08:43:24Z","receivedAt":"2018-10-13T08:43:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.10.18 um 04:46 schrieb Jeff King:\n> But no, right before that we have this line:\n> \n>            offset -= win->offset;\n> \n> So offset is in fact no longer its original meaning of \"offset into the\n> packfile\", but is now an offset of the specific request into the window\n> we found.\n> \n> So I think it's correct, but it sure confused me. I wonder if another\n> variable might help, like:\n> \n>    size_t offset_in_window;\n>    ...\n> \n>    /*\n>     * We know this difference will fit in a size_t, because our mmap\n>     * window by * definition can be no larger than a size_t.\n>     */\n>    offset_in_window = xsize_t(offset - win->offset);\n>    if (left)\n> \t*left = win->len - offset_in_window;\n>    return win->base + offset_in_window;\n> \n> I dunno. Maybe it is overkill.\n\nThank you for your analysis. No, I don't think that such a new variable \nwould be overkill. It is important to make such knowledge of value \nmagnitudes explicit precisely because it reduces confusion and helps \nreviewers of the code verify correctness.\n\n-- Hannes\n"},{"id":"360398","messageId":"fb6367b3-975b-1bc6-e95b-a457fb9e3a33@ramsayjones.plus.com","threadId":"49546","inReplyTo":"20181013050057.GA6805@tor.lan","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-14T02:16:36Z","receivedAt":"2018-10-14T02:40:56Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 13/10/18 06:00, Torsten Bögershausen wrote:\n> []\n>> Neither v1 nor v2 of this patch compiles on 32 bit Linux; see\n>>\n>>   https://travis-ci.org/git/git/jobs/440514375#L628\n>>\n>> The fixup patch below makes it compile on 32 bit and the test suite\n>> passes as well, but I didn't thoroughly review the changes; I only\n>> wanted to get 'pu' build again.\n>>\n> \n> Oh, yes, I didn't test under Linux 32 bit (and neither Windows)\n> I will try to compose a proper v3 the next days.\n\nI had a look at this today, and the result is given below.\n\nThe patch is effectively your v2 patch with SZEDER's fix-up\npatch on top! (I actually started with my own patch and 'pared\nit back' by removing the off_t -> size_t changes, etc; so I was\nsomewhat amused to note that the end result was effectively\nSZEDER's patch on top of your v2 ;-) ).\n\nI have tested this on 32- and 64-bit Linux, along with 64-bit\ncygwin (the test suite run hasn't finished yet, but I don't\nexpect any problem). I have an old Msys2 installation on Windows,\nwhich is the closest thing I have to a windows dev system, so I\nalso built this on MINGW32 and MINGW64 along with some light\nmanual testing (the test suite has never passed on Msys2 for me).\nThis is not the same as testing on Gfw, of course.\n\nATB,\nRamsay Jones\n\n-- >8 --\nFrom: Martin Koegler <martin.koegler@chello.at>\nSubject: [PATCH v3 1/1] zlib.c: use size_t for size\n\nSigned-off-by: Martin Koegler <martin.koegler@chello.at>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\nHelped-by: SZEDER Gábor <szeder.dev@gmail.com>\nSigned-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n---\n builtin/pack-objects.c | 11 ++++++-----\n cache.h                | 10 +++++-----\n pack-check.c           |  4 ++--\n packfile.c             |  4 ++--\n packfile.h             |  2 +-\n wrapper.c              |  8 ++++----\n zlib.c                 |  8 ++++----\n 7 files changed, 24 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex b059b86aee..3b5f2c38b3 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,\n \t\toff_t len)\n {\n \tunsigned char *in;\n-\tunsigned long avail;\n+\tsize_t avail;\n \n \twhile (len) {\n \t\tin = use_pack(p, w_curs, offset, &avail);\n \t\tif (avail > len)\n-\t\t\tavail = (unsigned long)len;\n+\t\t\tavail = xsize_t(len);\n \t\thashwrite(f, in, avail);\n \t\toffset += avail;\n \t\tlen -= avail;\n@@ -1529,8 +1529,8 @@ static void check_object(struct object_entry *entry)\n \t\tstruct pack_window *w_curs = NULL;\n \t\tconst unsigned char *base_ref = NULL;\n \t\tstruct object_entry *base_entry;\n-\t\tunsigned long used, used_0;\n-\t\tunsigned long avail;\n+\t\tsize_t used, used_0;\n+\t\tsize_t avail;\n \t\toff_t ofs;\n \t\tunsigned char *buf, c;\n \t\tenum object_type type;\n@@ -2002,7 +2002,8 @@ unsigned long oe_get_size_slow(struct packing_data *pack,\n \tstruct pack_window *w_curs;\n \tunsigned char *buf;\n \tenum object_type type;\n-\tunsigned long used, avail, size;\n+\tunsigned long used, size;\n+\tsize_t avail;\n \n \tif (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {\n \t\tread_lock();\ndiff --git a/cache.h b/cache.h\nindex 5d83022e3b..ba0ad73de1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -20,10 +20,10 @@\n #include <zlib.h>\n typedef struct git_zstream {\n \tz_stream z;\n-\tunsigned long avail_in;\n-\tunsigned long avail_out;\n-\tunsigned long total_in;\n-\tunsigned long total_out;\n+\tsize_t avail_in;\n+\tsize_t avail_out;\n+\tsize_t total_in;\n+\tsize_t total_out;\n \tunsigned char *next_in;\n \tunsigned char *next_out;\n } git_zstream;\n@@ -40,7 +40,7 @@ void git_deflate_end(git_zstream *);\n int git_deflate_abort(git_zstream *);\n int git_deflate_end_gently(git_zstream *);\n int git_deflate(git_zstream *, int flush);\n-unsigned long git_deflate_bound(git_zstream *, unsigned long);\n+size_t git_deflate_bound(git_zstream *, size_t);\n \n /* The length in bytes and in hex digits of an object name (SHA-1 value). */\n #define GIT_SHA1_RAWSZ 20\ndiff --git a/pack-check.c b/pack-check.c\nindex fa5f0ff8fa..d1e7f554ae 100644\n--- a/pack-check.c\n+++ b/pack-check.c\n@@ -33,7 +33,7 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n \tuint32_t data_crc = crc32(0, NULL, 0);\n \n \tdo {\n-\t\tunsigned long avail;\n+\t\tsize_t avail;\n \t\tvoid *data = use_pack(p, w_curs, offset, &avail);\n \t\tif (avail > len)\n \t\t\tavail = len;\n@@ -68,7 +68,7 @@ static int verify_packfile(struct packed_git *p,\n \n \tthe_hash_algo->init_fn(&ctx);\n \tdo {\n-\t\tunsigned long remaining;\n+\t\tsize_t remaining;\n \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n \t\toffset += remaining;\n \t\tif (!pack_sig_ofs)\ndiff --git a/packfile.c b/packfile.c\nindex f2850a00b5..013294aec7 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -585,7 +585,7 @@ static int in_window(struct pack_window *win, off_t offset)\n unsigned char *use_pack(struct packed_git *p,\n \t\tstruct pack_window **w_cursor,\n \t\toff_t offset,\n-\t\tunsigned long *left)\n+\t\tsize_t *left)\n {\n \tstruct pack_window *win = *w_cursor;\n \n@@ -1104,7 +1104,7 @@ int unpack_object_header(struct packed_git *p,\n \t\t\t unsigned long *sizep)\n {\n \tunsigned char *base;\n-\tunsigned long left;\n+\tsize_t left;\n \tunsigned long used;\n \tenum object_type type;\n \ndiff --git a/packfile.h b/packfile.h\nindex 6c4037605d..1fb482424b 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -78,7 +78,7 @@ extern void close_pack_index(struct packed_git *);\n \n extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);\n \n-extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);\n+extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, size_t *);\n extern void close_pack_windows(struct packed_git *);\n extern void close_pack(struct packed_git *);\n extern void close_all_packs(struct raw_object_store *o);\ndiff --git a/wrapper.c b/wrapper.c\nindex e4fa9d84cd..1a510bd6fc 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -67,11 +67,11 @@ static void *do_xmalloc(size_t size, int gentle)\n \t\t\tret = malloc(1);\n \t\tif (!ret) {\n \t\t\tif (!gentle)\n-\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t\t    (unsigned long)size);\n+\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n+\t\t\t\t    (uintmax_t)size);\n \t\t\telse {\n-\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n-\t\t\t\t      (unsigned long)size);\n+\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n+\t\t\t\t      (uintmax_t)size);\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t}\ndiff --git a/zlib.c b/zlib.c\nindex d594cba3fc..197a1acc7b 100644\n--- a/zlib.c\n+++ b/zlib.c\n@@ -29,7 +29,7 @@ static const char *zerr_to_string(int status)\n  */\n /* #define ZLIB_BUF_MAX ((uInt)-1) */\n #define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */\n-static inline uInt zlib_buf_cap(unsigned long len)\n+static inline uInt zlib_buf_cap(size_t len)\n {\n \treturn (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;\n }\n@@ -46,8 +46,8 @@ static void zlib_pre_call(git_zstream *s)\n \n static void zlib_post_call(git_zstream *s)\n {\n-\tunsigned long bytes_consumed;\n-\tunsigned long bytes_produced;\n+\tsize_t bytes_consumed;\n+\tsize_t bytes_produced;\n \n \tbytes_consumed = s->z.next_in - s->next_in;\n \tbytes_produced = s->z.next_out - s->next_out;\n@@ -150,7 +150,7 @@ int git_inflate(git_zstream *strm, int flush)\n #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)\n #endif\n \n-unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)\n+size_t git_deflate_bound(git_zstream *strm, size_t size)\n {\n \treturn deflateBound(&strm->z, size);\n }\n-- \n2.19.0\n\n"},{"id":"360399","messageId":"989a3ec9-d82d-45a1-8253-bf92ef18ce94@ramsayjones.plus.com","threadId":"49546","inReplyTo":"fb6367b3-975b-1bc6-e95b-a457fb9e3a33@ramsayjones.plus.com","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-14T02:31:10Z","receivedAt":"2018-10-14T02:40:56Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 14/10/18 03:16, Ramsay Jones wrote:\n> \n> \n> On 13/10/18 06:00, Torsten Bögershausen wrote:\n>> []\n>>> Neither v1 nor v2 of this patch compiles on 32 bit Linux; see\n>>>\n>>>   https://travis-ci.org/git/git/jobs/440514375#L628\n>>>\n>>> The fixup patch below makes it compile on 32 bit and the test suite\n>>> passes as well, but I didn't thoroughly review the changes; I only\n>>> wanted to get 'pu' build again.\n>>>\n>>\n>> Oh, yes, I didn't test under Linux 32 bit (and neither Windows)\n>> I will try to compose a proper v3 the next days.\n> \n> I had a look at this today, and the result is given below.\n> \n> The patch is effectively your v2 patch with SZEDER's fix-up\n> patch on top! (I actually started with my own patch and 'pared\n> it back' by removing the off_t -> size_t changes, etc; so I was\n> somewhat amused to note that the end result was effectively\n> SZEDER's patch on top of your v2 ;-) ).\n> \n> I have tested this on 32- and 64-bit Linux, along with 64-bit\n> cygwin (the test suite run hasn't finished yet, but I don't\n> expect any problem). I have an old Msys2 installation on Windows,\n> which is the closest thing I have to a windows dev system, so I\n> also built this on MINGW32 and MINGW64 along with some light\n> manual testing (the test suite has never passed on Msys2 for me).\n> This is not the same as testing on Gfw, of course.\n\nHo, Hum. Of course, I have just noticed that, similar to\ncopy_pack_data(), check_pack_crc() should probably use a\ncall to xsize_t(len) when assigning to avail.\n\nSorry about that. (I need some sleep now!)\n\nATB,\nRamsay Jones\n\n\n> -- >8 --\n> From: Martin Koegler <martin.koegler@chello.at>\n> Subject: [PATCH v3 1/1] zlib.c: use size_t for size\n> \n> Signed-off-by: Martin Koegler <martin.koegler@chello.at>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Torsten Bögershausen <tboegi@web.de>\n> Helped-by: SZEDER Gábor <szeder.dev@gmail.com>\n> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>\n> ---\n>  builtin/pack-objects.c | 11 ++++++-----\n>  cache.h                | 10 +++++-----\n>  pack-check.c           |  4 ++--\n>  packfile.c             |  4 ++--\n>  packfile.h             |  2 +-\n>  wrapper.c              |  8 ++++----\n>  zlib.c                 |  8 ++++----\n>  7 files changed, 24 insertions(+), 23 deletions(-)\n> \n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index b059b86aee..3b5f2c38b3 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,\n>  \t\toff_t len)\n>  {\n>  \tunsigned char *in;\n> -\tunsigned long avail;\n> +\tsize_t avail;\n>  \n>  \twhile (len) {\n>  \t\tin = use_pack(p, w_curs, offset, &avail);\n>  \t\tif (avail > len)\n> -\t\t\tavail = (unsigned long)len;\n> +\t\t\tavail = xsize_t(len);\n>  \t\thashwrite(f, in, avail);\n>  \t\toffset += avail;\n>  \t\tlen -= avail;\n> @@ -1529,8 +1529,8 @@ static void check_object(struct object_entry *entry)\n>  \t\tstruct pack_window *w_curs = NULL;\n>  \t\tconst unsigned char *base_ref = NULL;\n>  \t\tstruct object_entry *base_entry;\n> -\t\tunsigned long used, used_0;\n> -\t\tunsigned long avail;\n> +\t\tsize_t used, used_0;\n> +\t\tsize_t avail;\n>  \t\toff_t ofs;\n>  \t\tunsigned char *buf, c;\n>  \t\tenum object_type type;\n> @@ -2002,7 +2002,8 @@ unsigned long oe_get_size_slow(struct packing_data *pack,\n>  \tstruct pack_window *w_curs;\n>  \tunsigned char *buf;\n>  \tenum object_type type;\n> -\tunsigned long used, avail, size;\n> +\tunsigned long used, size;\n> +\tsize_t avail;\n>  \n>  \tif (e->type_ != OBJ_OFS_DELTA && e->type_ != OBJ_REF_DELTA) {\n>  \t\tread_lock();\n> diff --git a/cache.h b/cache.h\n> index 5d83022e3b..ba0ad73de1 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -20,10 +20,10 @@\n>  #include <zlib.h>\n>  typedef struct git_zstream {\n>  \tz_stream z;\n> -\tunsigned long avail_in;\n> -\tunsigned long avail_out;\n> -\tunsigned long total_in;\n> -\tunsigned long total_out;\n> +\tsize_t avail_in;\n> +\tsize_t avail_out;\n> +\tsize_t total_in;\n> +\tsize_t total_out;\n>  \tunsigned char *next_in;\n>  \tunsigned char *next_out;\n>  } git_zstream;\n> @@ -40,7 +40,7 @@ void git_deflate_end(git_zstream *);\n>  int git_deflate_abort(git_zstream *);\n>  int git_deflate_end_gently(git_zstream *);\n>  int git_deflate(git_zstream *, int flush);\n> -unsigned long git_deflate_bound(git_zstream *, unsigned long);\n> +size_t git_deflate_bound(git_zstream *, size_t);\n>  \n>  /* The length in bytes and in hex digits of an object name (SHA-1 value). */\n>  #define GIT_SHA1_RAWSZ 20\n> diff --git a/pack-check.c b/pack-check.c\n> index fa5f0ff8fa..d1e7f554ae 100644\n> --- a/pack-check.c\n> +++ b/pack-check.c\n> @@ -33,7 +33,7 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,\n>  \tuint32_t data_crc = crc32(0, NULL, 0);\n>  \n>  \tdo {\n> -\t\tunsigned long avail;\n> +\t\tsize_t avail;\n>  \t\tvoid *data = use_pack(p, w_curs, offset, &avail);\n>  \t\tif (avail > len)\n>  \t\t\tavail = len;\n> @@ -68,7 +68,7 @@ static int verify_packfile(struct packed_git *p,\n>  \n>  \tthe_hash_algo->init_fn(&ctx);\n>  \tdo {\n> -\t\tunsigned long remaining;\n> +\t\tsize_t remaining;\n>  \t\tunsigned char *in = use_pack(p, w_curs, offset, &remaining);\n>  \t\toffset += remaining;\n>  \t\tif (!pack_sig_ofs)\n> diff --git a/packfile.c b/packfile.c\n> index f2850a00b5..013294aec7 100644\n> --- a/packfile.c\n> +++ b/packfile.c\n> @@ -585,7 +585,7 @@ static int in_window(struct pack_window *win, off_t offset)\n>  unsigned char *use_pack(struct packed_git *p,\n>  \t\tstruct pack_window **w_cursor,\n>  \t\toff_t offset,\n> -\t\tunsigned long *left)\n> +\t\tsize_t *left)\n>  {\n>  \tstruct pack_window *win = *w_cursor;\n>  \n> @@ -1104,7 +1104,7 @@ int unpack_object_header(struct packed_git *p,\n>  \t\t\t unsigned long *sizep)\n>  {\n>  \tunsigned char *base;\n> -\tunsigned long left;\n> +\tsize_t left;\n>  \tunsigned long used;\n>  \tenum object_type type;\n>  \n> diff --git a/packfile.h b/packfile.h\n> index 6c4037605d..1fb482424b 100644\n> --- a/packfile.h\n> +++ b/packfile.h\n> @@ -78,7 +78,7 @@ extern void close_pack_index(struct packed_git *);\n>  \n>  extern uint32_t get_pack_fanout(struct packed_git *p, uint32_t value);\n>  \n> -extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, unsigned long *);\n> +extern unsigned char *use_pack(struct packed_git *, struct pack_window **, off_t, size_t *);\n>  extern void close_pack_windows(struct packed_git *);\n>  extern void close_pack(struct packed_git *);\n>  extern void close_all_packs(struct raw_object_store *o);\n> diff --git a/wrapper.c b/wrapper.c\n> index e4fa9d84cd..1a510bd6fc 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -67,11 +67,11 @@ static void *do_xmalloc(size_t size, int gentle)\n>  \t\t\tret = malloc(1);\n>  \t\tif (!ret) {\n>  \t\t\tif (!gentle)\n> -\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> -\t\t\t\t    (unsigned long)size);\n> +\t\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n> +\t\t\t\t    (uintmax_t)size);\n>  \t\t\telse {\n> -\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n> -\t\t\t\t      (unsigned long)size);\n> +\t\t\t\terror(\"Out of memory, malloc failed (tried to allocate %\" PRIuMAX \" bytes)\",\n> +\t\t\t\t      (uintmax_t)size);\n>  \t\t\t\treturn NULL;\n>  \t\t\t}\n>  \t\t}\n> diff --git a/zlib.c b/zlib.c\n> index d594cba3fc..197a1acc7b 100644\n> --- a/zlib.c\n> +++ b/zlib.c\n> @@ -29,7 +29,7 @@ static const char *zerr_to_string(int status)\n>   */\n>  /* #define ZLIB_BUF_MAX ((uInt)-1) */\n>  #define ZLIB_BUF_MAX ((uInt) 1024 * 1024 * 1024) /* 1GB */\n> -static inline uInt zlib_buf_cap(unsigned long len)\n> +static inline uInt zlib_buf_cap(size_t len)\n>  {\n>  \treturn (ZLIB_BUF_MAX < len) ? ZLIB_BUF_MAX : len;\n>  }\n> @@ -46,8 +46,8 @@ static void zlib_pre_call(git_zstream *s)\n>  \n>  static void zlib_post_call(git_zstream *s)\n>  {\n> -\tunsigned long bytes_consumed;\n> -\tunsigned long bytes_produced;\n> +\tsize_t bytes_consumed;\n> +\tsize_t bytes_produced;\n>  \n>  \tbytes_consumed = s->z.next_in - s->next_in;\n>  \tbytes_produced = s->z.next_out - s->next_out;\n> @@ -150,7 +150,7 @@ int git_inflate(git_zstream *strm, int flush)\n>  #define deflateBound(c,s)  ((s) + (((s) + 7) >> 3) + (((s) + 63) >> 6) + 11)\n>  #endif\n>  \n> -unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)\n> +size_t git_deflate_bound(git_zstream *strm, size_t size)\n>  {\n>  \treturn deflateBound(&strm->z, size);\n>  }\n> \n"},{"id":"360400","messageId":"20181014025207.GA1527@sigill.intra.peff.net","threadId":"49546","inReplyTo":"fb6367b3-975b-1bc6-e95b-a457fb9e3a33@ramsayjones.plus.com","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-14T02:52:07Z","receivedAt":"2018-10-14T02:55:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 14, 2018 at 03:16:36AM +0100, Ramsay Jones wrote:\n\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index b059b86aee..3b5f2c38b3 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,\n>  \t\toff_t len)\n>  {\n>  \tunsigned char *in;\n> -\tunsigned long avail;\n> +\tsize_t avail;\n>  \n>  \twhile (len) {\n>  \t\tin = use_pack(p, w_curs, offset, &avail);\n>  \t\tif (avail > len)\n> -\t\t\tavail = (unsigned long)len;\n> +\t\t\tavail = xsize_t(len);\n\nWe don't actually care about truncation here. The idea is that we take a\nbite-sized chunk via use_pack, and loop as necessary. So mod-2^32\ntruncation via a cast would be bad (we might not make forward progress),\nbut truncating to SIZE_MAX would be fine.\n\nThat said, we know that would not truncate here, because we must\nstrictly be shrinking \"avail\", which was already a size_t (unless \"len\"\nis negative, but then we are really screwed ;) ).\n\nSo I kind of wonder if a comment would be better than xsize_t here.\nSomething like:\n\n  if (avail > len) {\n\t/*\n\t * This can never truncate because we know that len is smaller\n\t * than avail, which is already a size_t.\n\t */\n\tavail = (size_t)len;\n  }\n\n-Peff\n"},{"id":"360409","messageId":"1e3233f4-ea40-6fff-0951-6b4885f88445@ramsayjones.plus.com","threadId":"49546","inReplyTo":"20181014025207.GA1527@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-14T15:03:48Z","receivedAt":"2018-10-14T15:03:53Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 14/10/18 03:52, Jeff King wrote:\n> On Sun, Oct 14, 2018 at 03:16:36AM +0100, Ramsay Jones wrote:\n> \n>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n>> index b059b86aee..3b5f2c38b3 100644\n>> --- a/builtin/pack-objects.c\n>> +++ b/builtin/pack-objects.c\n>> @@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,\n>>  \t\toff_t len)\n>>  {\n>>  \tunsigned char *in;\n>> -\tunsigned long avail;\n>> +\tsize_t avail;\n>>  \n>>  \twhile (len) {\n>>  \t\tin = use_pack(p, w_curs, offset, &avail);\n>>  \t\tif (avail > len)\n>> -\t\t\tavail = (unsigned long)len;\n>> +\t\t\tavail = xsize_t(len);\n> \n> We don't actually care about truncation here. The idea is that we take a\n> bite-sized chunk via use_pack, and loop as necessary. So mod-2^32\n> truncation via a cast would be bad (we might not make forward progress),\n> but truncating to SIZE_MAX would be fine.\n> \n> That said, we know that would not truncate here, because we must\n> strictly be shrinking \"avail\", which was already a size_t (unless \"len\"\n> is negative, but then we are really screwed ;) ).\n> \n> So I kind of wonder if a comment would be better than xsize_t here.\n> Something like:\n> \n>   if (avail > len) {\n> \t/*\n> \t * This can never truncate because we know that len is smaller\n> \t * than avail, which is already a size_t.\n> \t */\n> \tavail = (size_t)len;\n>   }\n\nHeh, you are, of course, correct! (that will learn me[1]). :-D\n\nHmm, let's see if I can muster the enthusiasm to do all that\ntesting again!\n\nATB,\nRamsay Jones\n\n[1] Since I started with my patch, when I had finished 'paring\nit back', the result didn't have this xsize_t() call. In order\nto make the result 'v2 + SZEDER's patch' (which I thought was\nquite neat) I added this call right at the end. :-P\n\n"},{"id":"360440","messageId":"20181015000147.GB13510@sigill.intra.peff.net","threadId":"49546","inReplyTo":"1e3233f4-ea40-6fff-0951-6b4885f88445@ramsayjones.plus.com","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-15T00:01:47Z","receivedAt":"2018-10-15T00:01:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 14, 2018 at 04:03:48PM +0100, Ramsay Jones wrote:\n\n> > So I kind of wonder if a comment would be better than xsize_t here.\n> > Something like:\n> > \n> >   if (avail > len) {\n> > \t/*\n> > \t * This can never truncate because we know that len is smaller\n> > \t * than avail, which is already a size_t.\n> > \t */\n> > \tavail = (size_t)len;\n> >   }\n> \n> Heh, you are, of course, correct! (that will learn me[1]). :-D\n> \n> Hmm, let's see if I can muster the enthusiasm to do all that\n> testing again!\n\nFor the record, I am not opposed to including the comment _and_ using\nxsize_t() to do the cast, giving us an assertion that the comment is\ncorrect.\n\n-Peff\n"},{"id":"360458","messageId":"d3d291ea-00d2-f0dd-5a43-cbea5476d64d@ramsayjones.plus.com","threadId":"49546","inReplyTo":"20181015000147.GB13510@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-10-15T00:41:19Z","receivedAt":"2018-10-15T00:41:25Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 15/10/18 01:01, Jeff King wrote:\n> On Sun, Oct 14, 2018 at 04:03:48PM +0100, Ramsay Jones wrote:\n> \n>>> So I kind of wonder if a comment would be better than xsize_t here.\n>>> Something like:\n>>>\n>>>   if (avail > len) {\n>>> \t/*\n>>> \t * This can never truncate because we know that len is smaller\n>>> \t * than avail, which is already a size_t.\n>>> \t */\n>>> \tavail = (size_t)len;\n>>>   }\n>>\n>> Heh, you are, of course, correct! (that will learn me[1]). :-D\n>>\n>> Hmm, let's see if I can muster the enthusiasm to do all that\n>> testing again!\n> \n> For the record, I am not opposed to including the comment _and_ using\n> xsize_t() to do the cast, giving us an assertion that the comment is\n> correct.\n\nHeh, I haven't found any enthusiasm tonight. Let's see if there\nare any more comments/opinions.\n\nThanks.\n\nATB,\nRamsay Jones\n\n"},{"id":"360480","messageId":"xmqqin236dpb.fsf@gitster-ct.c.googlers.com","threadId":"49546","inReplyTo":"d3d291ea-00d2-f0dd-5a43-cbea5476d64d@ramsayjones.plus.com","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-15T04:22:56Z","receivedAt":"2018-10-15T04:23:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n\n>> \n>> For the record, I am not opposed to including the comment _and_ using\n>> xsize_t() to do the cast, giving us an assertion that the comment is\n>> correct.\n>\n> Heh, I haven't found any enthusiasm tonight. Let's see if there\n> are any more comments/opinions.\n\nOK, in the meantime, I've replaced the thread-starter partch I had\nin 'pu' with your v3.\n\n"},{"id":"360485","messageId":"93518ecf-faf6-ce75-c081-8e2e7a56abb1@web.de","threadId":"49546","inReplyTo":"xmqqin236dpb.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/1] zlib.c: use size_t for size","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2018-10-15T05:54:57Z","receivedAt":"2018-10-15T05:55:22Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 15.10.18 06:22, Junio C Hamano wrote:\n> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:\n> \n>>>\n>>> For the record, I am not opposed to including the comment _and_ using\n>>> xsize_t() to do the cast, giving us an assertion that the comment is\n>>> correct.\n>>\n>> Heh, I haven't found any enthusiasm tonight. Let's see if there\n>> are any more comments/opinions.\n> \n> OK, in the meantime, I've replaced the thread-starter partch I had\n> in 'pu' with your v3.\n> \nIf that helps to move things forward:  I'm fully fine with v3.\n\n"}]}