threads / patch / 66738

patch, 3 partsuse size_t for a few sizes that are 32 bits everywhere

Subject: [PATCH 0/3] use size_t for a few sizes that are 32 bits everywhere

## tl;dr

Three patches widen types in hashwrite(), count_lines() and hash_chars(), found with -Wshorten-64-to-32 on a 64-bit Linux build. Read the story.

replies: 5people: 2as markdown or json

Elia Pinto· Nov 1, 2026, 15:36 UTC · lore

Unlike the previous series, these aren't about "long" on Windows: the types involved ("uint32_t", "int", "unsigned int") are 32 bits on every platform. They were found by scanning a native 64-bit Linux build with -Wshorten-64-to-32. That scan sees the "unsigned long" -> 32-bit conversions that a Windows target can't see, because there "long" is already 32 bits.

- 1/3 hashwrite(): a corrupt pack when pack-objects writes a compressed
  buffer of 4GiB or more in one call;
- 2/3 count_lines(): a wrong diff -B --stat for files of 2GiB or more;
- 3/3 hash_chars(): renames of files of 4GiB or more aren't recognised.

No tests, because they need GiB-sized inputs; each was verified by simulating the truncation at 16 bits.

The other results of that scan are bounded by the format or by the input (options, masks, line lengths, xdiff limited to 1023MiB). It doesn't depend on the other series in flight and merges cleanly with them. Based on master 50fe5cc63f.

Elia Pinto (3):
  csum-file: use size_t for the count passed to hashwrite()
  diff: use size_t for the size passed to count_lines()
  diffcore-delta: use size_t for the size of the buffer to hash
 csum-file.c      | 2 +-
 csum-file.h      | 2 +-
 diff.c           | 2 +-
 diffcore-delta.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
2.53.0
Elia Pinto· Nov 1, 2026, 15:36 UTC · re: Elia Pinto · lore

[PATCH 1/3] csum-file: use size_t for the count passed to hashwrite()

hashwrite() takes the byte count as "uint32_t" on all platforms. When pack-objects writes the entire compressed buffer of an object in a single call, which can exceed 4GiB if the object is in memory, poorly compressible and core.bigFileThreshold is raised accordingly, it writes only "count mod 2^32" bytes, corrupting the pack.

This was verified using a 16-bit simulation, where "git cat-file" and "git fsck" failed after "git repack -a -d" with "offset beyond end of packfile (truncated pack?)".

Make the count parameter of hashwrite() a "size_t". The internal loop already handles writing in buffer-sized blocks.

Triggering this requires processing an object of 4GiB or more, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 csum-file.c | 2 +-
 csum-file.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
Show changes to 2 files +2 −2

csum-file.c, csum-file.h

diff --git a/csum-file.c b/csum-file.c
index fe18ee1de3..26aa662637 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -102,7 +102,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result,
 	return fd;
 }
 
-void hashwrite(struct hashfile *f, const void *buf, uint32_t count)
+void hashwrite(struct hashfile *f, const void *buf, size_t count)
 {
 	while (count) {
 		unsigned left = f->buffer_len - f->offset;
diff --git a/csum-file.h b/csum-file.h
index 6ed74d1637..f1c8a9df13 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -75,7 +75,7 @@ void free_hashfile(struct hashfile *f);
  * Finalize the hashfile by flushing data to disk and free'ing it.
  */
 int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int);
-void hashwrite(struct hashfile *, const void *, uint32_t);
+void hashwrite(struct hashfile *, const void *, size_t);
 void hashflush(struct hashfile *f);
 void crc32_begin(struct hashfile *);
 uint32_t crc32_end(struct hashfile *);

-- 
2.53.0
Elia Pinto· Nov 1, 2026, 15:36 UTC · re: Elia Pinto · lore

[PATCH 2/3] diff: use size_t for the size passed to count_lines()

count_lines() takes the size as an "int", whereas callers have "unsigned long" or "size_t". It is used for the diffstat of complete rewrites ("git diff -B --stat") and in emit_rewrite_diff().

With a file of 2GiB or more, the "int" becomes negative (or truncated above 4GiB), leading to an incorrect line count. This only affects the diffstat output, with no memory access issues.

This was verified using a 16-bit simulation, where "git diff -B --stat" reported 0 changed lines instead of the actual count.

Make the size parameter a size_t. The line counter remains "int".

Triggering this requires processing a file of 2GiB or more, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to diff.c +1 −1
diff --git a/diff.c b/diff.c
index b4ac17f8ef..ed7a931c41 100644
--- a/diff.c
+++ b/diff.c
@@ -655,7 +655,7 @@ struct line_range_callback {
 	int ret;			/* latched error from orig_line_fn */
 };
 
-static int count_lines(const char *data, int size)
+static int count_lines(const char *data, size_t size)
 {
 	int count, ch, completely_empty = 1, nl_just_seen = 0;
 	count = 0;

-- 
2.53.0
Phillip Wood· Nov 4, 2026, 13:58 UTC · re: Elia Pinto · lore

Re: [PATCH 2/3] diff: use size_t for the size passed to count_lines()

Hi Elia
On 01/11/2026 15:36, Elia Pinto wrote:
Show 9 quoted lines
> count_lines() takes the size as an "int", whereas callers have "unsigned
> long" or "size_t". It is used for the diffstat of complete rewrites
> ("git diff -B --stat") and in emit_rewrite_diff().
> > With a file of 2GiB or more, the "int" becomes negative (or truncated
> above 4GiB), leading to an incorrect line count. This only affects the
> diffstat output, with no memory access issues.
> > This was verified using a 16-bit simulation, where "git diff -B --stat"
> reported 0 changed lines instead of the actual count.
> > Make the size parameter a size_t. The line counter remains "int".
That means the counter is subject to overflow as a file can contain as many lines as its size.
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.
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.
Thanks
Phillip
Show 18 quoted lines
> Triggering this requires processing a file of 2GiB or more, so no test
> is added.
> > Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
> ---
>   diff.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> > diff --git a/diff.c b/diff.c
> index b4ac17f8ef..ed7a931c41 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -655,7 +655,7 @@ struct line_range_callback {
>   	int ret;			/* latched error from orig_line_fn */
>   };
>   > -static int count_lines(const char *data, int size)
> +static int count_lines(const char *data, size_t size)
>   {
>   	int count, ch, completely_empty = 1, nl_just_seen = 0;
>   	count = 0;
Phillip Wood· Nov 4, 2026, 15:45 UTC · re: Phillip Wood · lore

Re: [PATCH 2/3] diff: use size_t for the size passed to count_lines()

On 04/11/2026 13:58, Phillip Wood wrote:
Show 15 quoted lines
> On 01/11/2026 15:36, Elia Pinto wrote:
>> count_lines() takes the size as an "int", whereas callers have "unsigned
>> long" or "size_t". It is used for the diffstat of complete rewrites
>> ("git diff -B --stat") and in emit_rewrite_diff().
>>
>> With a file of 2GiB or more, the "int" becomes negative (or truncated
>> above 4GiB), leading to an incorrect line count. This only affects the
>> diffstat output, with no memory access issues.
>>
>> This was verified using a 16-bit simulation, where "git diff -B --stat"
>> reported 0 changed lines instead of the actual count.
>>
>  > Make the size parameter a size_t. The line counter remains "int".
> > That means the counter is subject to overflow as a file can contain as > many lines as its size.
> > 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.
Sorry 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.
Phillip
Show 24 quoted lines
> 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.
> > Thanks
> > Phillip
> > >> Triggering this requires processing a file of 2GiB or more, so no test
>> is added.
>>
>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>> ---
>>   diff.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/diff.c b/diff.c
>> index b4ac17f8ef..ed7a931c41 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -655,7 +655,7 @@ struct line_range_callback {
>>       int ret;            /* latched error from orig_line_fn */
>>   };
>> -static int count_lines(const char *data, int size)
>> +static int count_lines(const char *data, size_t size)
>>   {
>>       int count, ch, completely_empty = 1, nl_just_seen = 0;
>>       count = 0;
>
Elia Pinto· Nov 1, 2026, 15:36 UTC · re: Elia Pinto · lore

[PATCH 3/3] diffcore-delta: use size_t for the size of the buffer to hash

hash_chars() in diffcore-delta.c takes the buffer size as an "unsigned int". With a file of 4GiB or more, it hashes only a prefix; because the similarity score is calculated against the full size, similarity is undervalued, so true renames are missed, which also affects "git log --follow" and merges.

This was verified using a 16-bit simulation, where "git diff --cached -M" reported a delete and add instead of a rename (e.g., "D a" and "A b" instead of "R099 a b").

Make the size variable "size_t".

Triggering this requires processing a file of 4GiB or more, so no test is added.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diffcore-delta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to diffcore-delta.c +1 −1
diff --git a/diffcore-delta.c b/diffcore-delta.c
index 2b7db39983..7f10f79ffe 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -127,7 +127,7 @@ static struct spanhash_top *hash_chars(struct repository *r,
 	unsigned int accum1, accum2, hashval;
 	struct spanhash_top *hash;
 	unsigned char *buf = one->data;
-	unsigned int sz = one->size;
+	size_t sz = one->size;
 	int is_text = !diff_filespec_is_binary(r, one);
 
 	i = INITIAL_HASH_SIZE;

-- 
2.53.0

← back to recent threads