{"thread":{"id":"66738","subject":"[PATCH 0/3] use size_t for a few sizes that are 32 bits everywhere","startedAt":"2026-11-01T15:36:48Z","lastAt":"2026-11-04T15:45:47Z","messageCount":6,"participants":["Elia Pinto","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"557080","messageId":"cover.1793546752.git.gitter.spiros@gmail.com","threadId":"66738","inReplyTo":null,"subject":"[PATCH 0/3] use size_t for a few sizes that are 32 bits everywhere","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2026-11-01T15:36:48Z","receivedAt":"2026-11-01T15:36:48Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"Unlike the previous series, these aren't about \"long\" on Windows: the\ntypes involved (\"uint32_t\", \"int\", \"unsigned int\") are 32 bits on every\nplatform. They were found by scanning a native 64-bit Linux build with\n-Wshorten-64-to-32. That scan sees the \"unsigned long\" -> 32-bit\nconversions that a Windows target can't see, because there \"long\" is\nalready 32 bits.\n\n- 1/3 hashwrite(): a corrupt pack when pack-objects writes a compressed\n buffer of 4GiB or more in one call;\n- 2/3 count_lines(): a wrong diff -B --stat for files of 2GiB or more;\n- 3/3 hash_chars(): renames of files of 4GiB or more aren't recognised.\n\nNo tests, because they need GiB-sized inputs; each was verified by\nsimulating the truncation at 16 bits.\n\nThe other results of that scan are bounded by the format or by the input\n(options, masks, line lengths, xdiff limited to 1023MiB). It doesn't\ndepend on the other series in flight and merges cleanly with them. Based\non master 50fe5cc63f.\n\nElia Pinto (3):\n csum-file: use size_t for the count passed to hashwrite()\n diff: use size_t for the size passed to count_lines()\n diffcore-delta: use size_t for the size of the buffer to hash\n\n csum-file.c | 2 +-\n csum-file.h | 2 +-\n diff.c | 2 +-\n diffcore-delta.c | 2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\n\nbase-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7\n-- \n2.53.0\n\n\n"},{"id":"557081","messageId":"07baad46b153b255b6e70f2597f1bf41b9c48dfd.1793546752.git.gitter.spiros@gmail.com","threadId":"66738","inReplyTo":"cover.1793546752.git.gitter.spiros@gmail.com","subject":"[PATCH 1/3] csum-file: use size_t for the count passed to hashwrite()","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2026-11-01T15:36:49Z","receivedAt":"2026-11-01T15:36:49Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"hashwrite() takes the byte count as \"uint32_t\" on all platforms. When\npack-objects writes the entire compressed buffer of an object in a\nsingle call, which can exceed 4GiB if the object is in memory, poorly\ncompressible and core.bigFileThreshold is raised accordingly, it writes\nonly \"count mod 2^32\" bytes, corrupting the pack.\n\nThis was verified using a 16-bit simulation, where \"git cat-file\" and\n\"git fsck\" failed after \"git repack -a -d\" with \"offset beyond end of\npackfile (truncated pack?)\".\n\nMake the count parameter of hashwrite() a \"size_t\". The internal loop\nalready handles writing in buffer-sized blocks.\n\nTriggering this requires processing an object of 4GiB or more, so no\ntest is added.\n\nSigned-off-by: Elia Pinto \n---\n csum-file.c | 2 +-\n csum-file.h | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/csum-file.c b/csum-file.c\nindex fe18ee1de3..26aa662637 100644\n--- a/csum-file.c\n+++ b/csum-file.c\n@@ -102,7 +102,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result,\n \treturn fd;\n }\n \n-void hashwrite(struct hashfile *f, const void *buf, uint32_t count)\n+void hashwrite(struct hashfile *f, const void *buf, size_t count)\n {\n \twhile (count) {\n \t\tunsigned left = f->buffer_len - f->offset;\ndiff --git a/csum-file.h b/csum-file.h\nindex 6ed74d1637..f1c8a9df13 100644\n--- a/csum-file.h\n+++ b/csum-file.h\n@@ -75,7 +75,7 @@ void free_hashfile(struct hashfile *f);\n * Finalize the hashfile by flushing data to disk and free'ing it.\n */\n int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int);\n-void hashwrite(struct hashfile *, const void *, uint32_t);\n+void hashwrite(struct hashfile *, const void *, size_t);\n void hashflush(struct hashfile *f);\n void crc32_begin(struct hashfile *);\n uint32_t crc32_end(struct hashfile *);\n-- \n2.53.0\n\n\n"},{"id":"557082","messageId":"2011b90d80c5a823878055fd1016bc38b6a072b7.1793546752.git.gitter.spiros@gmail.com","threadId":"66738","inReplyTo":"cover.1793546752.git.gitter.spiros@gmail.com","subject":"[PATCH 2/3] diff: use size_t for the size passed to count_lines()","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2026-11-01T15:36:50Z","receivedAt":"2026-11-01T15:36:50Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"count_lines() takes the size as an \"int\", whereas callers have \"unsigned\nlong\" or \"size_t\". It is used for the diffstat of complete rewrites\n(\"git diff -B --stat\") and in emit_rewrite_diff().\n\nWith a file of 2GiB or more, the \"int\" becomes negative (or truncated\nabove 4GiB), leading to an incorrect line count. This only affects the\ndiffstat output, with no memory access issues.\n\nThis was verified using a 16-bit simulation, where \"git diff -B --stat\"\nreported 0 changed lines instead of the actual count.\n\nMake the size parameter a size_t. The line counter remains \"int\".\n\nTriggering this requires processing a file of 2GiB or more, so no test\nis added.\n\nSigned-off-by: Elia Pinto \n---\n diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex b4ac17f8ef..ed7a931c41 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -655,7 +655,7 @@ struct line_range_callback {\n \tint ret;\t\t\t/* latched error from orig_line_fn */\n };\n \n-static int count_lines(const char *data, int size)\n+static int count_lines(const char *data, size_t size)\n {\n \tint count, ch, completely_empty = 1, nl_just_seen = 0;\n \tcount = 0;\n-- \n2.53.0\n\n\n"},{"id":"557083","messageId":"0c5442280cc441ffa3ed61af74466e2b4d2d6794.1793546752.git.gitter.spiros@gmail.com","threadId":"66738","inReplyTo":"cover.1793546752.git.gitter.spiros@gmail.com","subject":"[PATCH 3/3] diffcore-delta: use size_t for the size of the buffer to hash","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2026-11-01T15:36:51Z","receivedAt":"2026-11-01T15:36:51Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"hash_chars() in diffcore-delta.c takes the buffer size as an \"unsigned\nint\". With a file of 4GiB or more, it hashes only a prefix; because the\nsimilarity score is calculated against the full size, similarity is\nundervalued, so true renames are missed, which also affects\n\"git log --follow\" and merges.\n\nThis was verified using a 16-bit simulation, where\n\"git diff --cached -M\" reported a delete and add instead of a rename\n(e.g., \"D a\" and \"A b\" instead of \"R099 a b\").\n\nMake the size variable \"size_t\".\n\nTriggering this requires processing a file of 4GiB or more, so no test\nis added.\n\nSigned-off-by: Elia Pinto \n---\n diffcore-delta.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diffcore-delta.c b/diffcore-delta.c\nindex 2b7db39983..7f10f79ffe 100644\n--- a/diffcore-delta.c\n+++ b/diffcore-delta.c\n@@ -127,7 +127,7 @@ static struct spanhash_top *hash_chars(struct repository *r,\n \tunsigned int accum1, accum2, hashval;\n \tstruct spanhash_top *hash;\n \tunsigned char *buf = one->data;\n-\tunsigned int sz = one->size;\n+\tsize_t sz = one->size;\n \tint is_text = !diff_filespec_is_binary(r, one);\n \n \ti = INITIAL_HASH_SIZE;\n-- \n2.53.0\n\n\n"},{"id":"557439","messageId":"0cb42395-3e62-42e0-bfd3-a4265ed7cc33@gmail.com","threadId":"66738","inReplyTo":"2011b90d80c5a823878055fd1016bc38b6a072b7.1793546752.git.gitter.spiros@gmail.com","subject":"Re: [PATCH 2/3] diff: use size_t for the size passed to count_lines()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-11-04T13:58:29Z","receivedAt":"2026-11-04T13:58:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Elia\n\nOn 01/11/2026 15:36, Elia Pinto wrote:\n> count_lines() takes the size as an \"int\", whereas callers have \"unsigned\n> long\" or \"size_t\". It is used for the diffstat of complete rewrites\n> (\"git diff -B --stat\") and in emit_rewrite_diff().\n> > With a file of 2GiB or more, the \"int\" becomes negative (or truncated\n> above 4GiB), leading to an incorrect line count. This only affects the\n> diffstat output, with no memory access issues.\n> > This was verified using a 16-bit simulation, where \"git diff -B --stat\"\n> reported 0 changed lines instead of the actual count.\n> > Make the size parameter a size_t. The line counter remains \"int\".\n\nThat means the counter is subject to overflow as a file can contain as many lines as its size.\n\nI don't know what the simulation entailed, but this patch changes the return type without changing either the type of the returned value which can now overflow, or the types where the return values is stored by callers.\n\nI'm concerned that the preparation of these patches and the others you've sent has relied too heavily on a flawed mechanical process, and have not enough on manual review reading the code and reasoning about it.\n\nThanks\n\nPhillip\n\n\n> Triggering this requires processing a file of 2GiB or more, so no test\n> is added.\n> > Signed-off-by: Elia Pinto \n> ---\n> diff.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> > diff --git a/diff.c b/diff.c\n> index b4ac17f8ef..ed7a931c41 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -655,7 +655,7 @@ struct line_range_callback {\n> \tint ret;\t\t\t/* latched error from orig_line_fn */\n> };\n> > -static int count_lines(const char *data, int size)\n> +static int count_lines(const char *data, size_t size)\n> {\n> \tint count, ch, completely_empty = 1, nl_just_seen = 0;\n> \tcount = 0;\n\n\n"},{"id":"557446","messageId":"0b3f8998-bb3d-4a91-822d-291562691a6f@gmail.com","threadId":"66738","inReplyTo":"0cb42395-3e62-42e0-bfd3-a4265ed7cc33@gmail.com","subject":"Re: [PATCH 2/3] diff: use size_t for the size passed to count_lines()","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-11-04T15:45:47Z","receivedAt":"2026-11-04T15:45:47Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 04/11/2026 13:58, Phillip Wood wrote:\n> On 01/11/2026 15:36, Elia Pinto wrote:\n>> count_lines() takes the size as an \"int\", whereas callers have \"unsigned\n>> long\" or \"size_t\". It is used for the diffstat of complete rewrites\n>> (\"git diff -B --stat\") and in emit_rewrite_diff().\n>>\n>> With a file of 2GiB or more, the \"int\" becomes negative (or truncated\n>> above 4GiB), leading to an incorrect line count. This only affects the\n>> diffstat output, with no memory access issues.\n>>\n>> This was verified using a 16-bit simulation, where \"git diff -B --stat\"\n>> reported 0 changed lines instead of the actual count.\n>>\n> > Make the size parameter a size_t. The line counter remains \"int\".\n> > That means the counter is subject to overflow as a file can contain as > many lines as its size.\n> > I don't know what the simulation entailed, but this patch changes the > return type without changing either the type of the returned value which > can now overflow, or the types where the return values is stored by > callers.\n\nSorry I seem to have misread the patch when I wrote that - the return type is not changed. We're still left with the possibility of the count overflowing now that \"size\" is a size_t though I think.\n\nPhillip\n\n> I'm concerned that the preparation of these patches and the others > you've sent has relied too heavily on a flawed mechanical process, and > have not enough on manual review reading the code and reasoning about it.\n> > Thanks\n> > Phillip\n> > >> Triggering this requires processing a file of 2GiB or more, so no test\n>> is added.\n>>\n>> Signed-off-by: Elia Pinto \n>> ---\n>>   diff.c | 2 +-\n>>   1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/diff.c b/diff.c\n>> index b4ac17f8ef..ed7a931c41 100644\n>> --- a/diff.c\n>> +++ b/diff.c\n>> @@ -655,7 +655,7 @@ struct line_range_callback {\n>>       int ret;            /* latched error from orig_line_fn */\n>>   };\n>> -static int count_lines(const char *data, int size)\n>> +static int count_lines(const char *data, size_t size)\n>>   {\n>>       int count, ch, completely_empty = 1, nl_just_seen = 0;\n>>       count = 0;\n> "}]}