From: Phillip Wood Date: Wed, 04 Nov 2026 15:45:47 GMT Subject: Re: [PATCH 2/3] diff: use size_t for the size passed to count_lines() Message-ID: <0b3f8998-bb3d-4a91-822d-291562691a6f@gmail.com> In-Reply-To: <0cb42395-3e62-42e0-bfd3-a4265ed7cc33@gmail.com> On 04/11/2026 13:58, Phillip Wood wrote: > 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 > 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 >> --- >>   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; >