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;