[PATCH 0/3] use size_t for a few more object and buffer sizes

[PATCH 0/3] use size_t for a few more object and buffer sizes

[Elia Pinto]

On 64-bit Windows, "long" is still 32 bits wide. Whenever a size_t
travels through an "unsigned long", anything above 4 GiB is silently
lost. Johannes's js/objects-larger-than-4gb-on-windows work fixed the
read side of the object database, and Peff's jk/xdiff-size_t did the
same for the diff and merge machinery. This series picks up three
places that were left narrow.

 - odb: the write API still takes the object length as unsigned long.
   A large file that is converted before being stored (for example
   with core.autocrlf, which Git for Windows enables by default) can
   reach odb_write_object() with a size_t length above 4 GiB. The
   blob is then written truncated, with no error, and the index
   happily records it.

 - diff: diff_words_fill() grows its buffer with a signed "long"
   allocation size. Its output can be twice its input, and on LLP64
   the growth computation overflows past about 700MiB, which is well
   within what xdiff accepts. In practice Git dies on a huge
   allocation request.

 - convert: the helpers that decide whether a file is binary, has
   CRLF line endings or "$Id$" keywords take an unsigned long length.
   For files over 4GiB they only look at a prefix. In the same area,
   read_blob_data_from_index() narrows its result with
   cast_size_t_to_ulong(), so "git add" with core.autocrlf dies when
   the staged version is that large.

There are no new tests. Each case needs files of several GiB and a
32-bit "long", and the commit messages explain how to reproduce them.

How these were found
--------------------

I wanted a list of all the places where a 64-bit size is narrowed on
Windows, rather than waiting for the next bug report. Linux compilers
cannot see the problem, because there "long" and "size_t" have the
same width. So I analysed the tree as a Windows target instead.

The first step is a compilation database from a real MinGW
configuration. Forcing --target onto a database generated on Linux
does not work well: Linux-only defines leak in, regex.h is missing
from MinGW, and the precompiled header is rejected for the wrong
target. A Meson cross build avoids all of that:

  $ cat mingw64.cross
  [binaries]
  c       = 'x86_64-w64-mingw32-gcc'
  ar      = 'x86_64-w64-mingw32-ar'
  strip   = 'x86_64-w64-mingw32-strip'
  windres = 'x86_64-w64-mingw32-windres'

  [host_machine]
  system     = 'windows'
  cpu_family = 'x86_64'
  cpu        = 'x86_64'
  endian     = 'little'

  $ meson setup build-win --cross-file mingw64.cross \
	-Db_pch=false -Dsane_tool_path=/usr/bin \
	-Dcurl=disabled -Dexpat=disabled -Dpcre2=disabled \
	-Dgettext=disabled -Diconv=disabled -Drust=disabled
  $ meson compile -C build-win

Two of these options are not obvious. Without -Dsane_tool_path,
setup stops because meson.build adds "C:/Program Files/Git/bin" to
the program search path, which is not absolute on a Linux host. And
-Db_pch=false is needed because clang cannot read the precompiled
header that GCC produces. The build only has to get far enough to
generate the headers; link errors do not matter here.

The second step runs clang-tidy over that database with
-Wshorten-64-to-32, for the x86_64-w64-mingw32 target, skipping the
bundled zlib and OpenSSL. One detail cost me a while: with
"-checks=-*", clang-tidy drops compiler warnings unless
clang-diagnostic-shorten-64-to-32 is enabled explicitly. I wrapped
all this in a small script, which also refuses to report "no
findings" when some translation units failed to parse:

  https://github.com/devzero2000/toyscript/blob/main/check_llp64_portability.sh

It is not meant for inclusion. I mention it so that the numbers below
can be reproduced, and in case someone finds it useful.

What the scan shows
-------------------

All 569 translation units were analysed, none failed. There are 1432
64-to-32 truncation warnings, but most of them are not specific to
Windows: about 1200 narrow a size_t to an int, as they do on Linux.
Only 237 have "long" or "unsigned long" as their destination.

Most of those 237 sit on API boundaries that are known and that
commit c6a4629e32 names explicitly: diff, blame, textconv and
fast-import. Others are in tree walking, object parsing and the pack
tools, where an oversized object makes Git die rather than misbehave.
I would like to convert them in later small series, one area at a
time, in the same style as jk/xdiff-size_t.

Johannes, the first patch continues your write-side work. I hope it
does not step on a follow-up you already have in progress.

Elia Pinto (3):
  odb: use size_t for the length of objects to write
  diff: use size_t for the allocation size of word-diff buffers
  convert: use size_t for buffer sizes

 attr.c                |  2 +-
 convert.c             | 14 +++++++-------
 diff.c                |  4 ++--
 odb.c                 |  2 +-
 odb.h                 |  4 ++--
 odb/source-inmemory.c |  2 +-
 odb/source-loose.c    |  2 +-
 odb/source.h          |  2 +-
 read-cache-ll.h       |  2 +-
 read-cache.c          |  4 ++--
 10 files changed, 19 insertions(+), 19 deletions(-)


base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926
--
2.53.0

[PATCH 1/3] odb: use size_t for the length of objects to write

[Elia Pinto]

c6a4629e32 (odb: use size_t for object_info.sizep and the size APIs,
2026-06-15) widened the read side of the object database API, and the
write_object() callback of odb_source already takes a size_t. But
odb_write_object_ext(), odb_write_object() and odb_source_write_object()
in front of it, as well as write_loose_object() behind it, still take
an "unsigned long", which is only 32 bits wide on Windows.

Most callers hand them a size_t from a strbuf or from odb_read_object(),
which silently gets truncated. The most visible case is index_mem() and
index_stream_convert_blob(): when "git add" or "git hash-object -w"
runs a file over 4GiB through a conversion (core.autocrlf, which is on
by default in Git for Windows, an eol or ident attribute, or a clean
filter), we write a blob made of the first "size mod 2^32" bytes. The
index records it with the stat data of the real file, so "git status"
stays clean and nobody notices until the file is checked out again.
"git hash-object" without "-w" uses hash_object_file(), which takes a
size_t, so the two disagree on the object ID of the same file.

Use size_t throughout, including for the size kept by the in-memory
source.

The existing "hash filtered files over 4GB correctly" test in t1007
only hashes. Running the same with "git hash-object -w" on Windows
shows the problem, but needs another pass over a 5GiB file, so no new
test is added here.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 odb.c                 | 2 +-
 odb.h                 | 4 ++--
 odb/source-inmemory.c | 2 +-
 odb/source-loose.c    | 2 +-
 odb/source.h          | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index 98c5c862f5..6e9daf11e4 100644
--- a/odb.c
+++ b/odb.c
@@ -667,7 +667,7 @@ void odb_assert_oid_type(struct object_database *odb,
 }
 
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid_in,
diff --git a/odb.h b/odb.h
index 0ccc47ee73..25d02fae75 100644
--- a/odb.h
+++ b/odb.h
@@ -633,14 +633,14 @@ enum odb_write_object_flags {
  * Returns 0 on success, a negative error code otherwise.
  */
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid,
 			 enum odb_write_object_flags flags);
 
 static inline int odb_write_object(struct object_database *odb,
-				   const void *buf, unsigned long len,
+				   const void *buf, size_t len,
 				   enum object_type type,
 				   struct object_id *oid)
 {
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index 868e723c81..ab7173cd2d 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -10,7 +10,7 @@
 struct inmemory_object {
 	enum object_type type;
 	const void *buf;
-	unsigned long size;
+	size_t size;
 };
 
 static const struct inmemory_object *find_cached_object(struct odb_source_inmemory *source,
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 0664634938..fe1a298364 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -770,7 +770,7 @@ static int end_loose_object_common(struct odb_source_loose *loose,
 
 static int write_loose_object(struct odb_source_loose *loose,
 			      const struct object_id *oid, char *hdr,
-			      int hdrlen, const void *buf, unsigned long len,
+			      int hdrlen, const void *buf, size_t len,
 			      const time_t *mtime, unsigned flags)
 {
 	int fd, ret;
diff --git a/odb/source.h b/odb/source.h
index 1867347dfc..8e6ed96399 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -469,7 +469,7 @@ static inline int odb_source_freshen_object(struct odb_source *source,
  * object ID and the compatibility object ID, if non-NULL.
  */
 static inline int odb_source_write_object(struct odb_source *source,
-					  const void *buf, unsigned long len,
+					  const void *buf, size_t len,
 					  enum object_type type,
 					  const struct object_id *oid,
 					  const struct object_id *compat_oid,
-- 
2.53.0

Re: [PATCH 1/3] odb: use size_t for the length of objects to write

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 12:16:37PM +0200, Elia Pinto wrote:
> c6a4629e32 (odb: use size_t for object_info.sizep and the size APIs,
> 2026-06-15) widened the read side of the object database API, and the
> write_object() callback of odb_source already takes a size_t. But
> odb_write_object_ext(), odb_write_object() and odb_source_write_object()
> in front of it, as well as write_loose_object() behind it, still take
> an "unsigned long", which is only 32 bits wide on Windows.

Sometimes I wish that C would give us an easier way to detect such
mismatches. It of course does have optional warnings, but enabling those
now would probably lead to thousands of warnings.

> Most callers hand them a size_t from a strbuf or from odb_read_object(),
> which silently gets truncated. The most visible case is index_mem() and
> index_stream_convert_blob(): when "git add" or "git hash-object -w"
> runs a file over 4GiB through a conversion (core.autocrlf, which is on
> by default in Git for Windows, an eol or ident attribute, or a clean
> filter), we write a blob made of the first "size mod 2^32" bytes. The
> index records it with the stat data of the real file, so "git status"
> stays clean and nobody notices until the file is checked out again.
> "git hash-object" without "-w" uses hash_object_file(), which takes a
> size_t, so the two disagree on the object ID of the same file.

Awesome.

> Use size_t throughout, including for the size kept by the in-memory
> source.
> 
> The existing "hash filtered files over 4GB correctly" test in t1007
> only hashes. Running the same with "git hash-object -w" on Windows
> shows the problem, but needs another pass over a 5GiB file, so no new
> test is added here.

But wouldn't it make sense to add a test? It's a rather serious bug, and
the existing tests are marked with EXPENSIVE already anyway. So I don't
think this is a good enough argument to not add another EXPENSIVE test.

> diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
> index 868e723c81..ab7173cd2d 100644
> --- a/odb/source-inmemory.c
> +++ b/odb/source-inmemory.c
> @@ -10,7 +10,7 @@
>  struct inmemory_object {
>  	enum object_type type;
>  	const void *buf;
> -	unsigned long size;
> +	size_t size;
>  };

I really hope nobody writes 4GB blobs into memory :) But despite that,
this fix is correct anyway.

Patrick

[PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers

[Elia Pinto]

Since 3c7a31204f (xdiff: use size_t for buffer sizes, 2026-09-30) the
size of an mmfile_t is a size_t, but the word-diff code still grows
these buffers with ALLOC_GROW() using an "unsigned long" (in struct
diff_words_buffer) or a "long" (in diff_words_fill()) for the allocated
size.

On platforms where "long" is 32 bits wide this is not just cosmetic.
diff_words_fill() writes every word followed by a newline, so its
output can be up to twice as large as its input, which is limited by
MAX_XDIFF_SIZE to just under 1GiB. alloc_nr() computes (alloc + 16) * 3
in the type of "alloc", which overflows a signed 32-bit "long" as soon
as the buffer grows past about 700MiB. The result is undefined behavior,
in practice a negative size that REALLOC_ARRAY() turns into a huge
allocation request and a die().

The buffer in diff_words_append() is bounded by MAX_XDIFF_SIZE, so
there it is only a matter of consistency, but if that limit ever went
away, ALLOC_GROW() would truncate the requested size, and the memcpy()
right after it would overflow the buffer.

Use size_t for both.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index b4ac17f8ef..9483931c05 100644
--- a/diff.c
+++ b/diff.c
@@ -1959,7 +1959,7 @@ static void emit_rewrite_diff(const char *name_a,
 
 struct diff_words_buffer {
 	mmfile_t text;
-	unsigned long alloc;
+	size_t alloc;
 	struct diff_words_orig {
 		const char *begin, *end;
 	} *orig;
@@ -2192,7 +2192,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
 		regex_t *word_regex)
 {
 	int i, j;
-	long alloc = 0;
+	size_t alloc = 0;
 
 	out->size = 0;
 	out->ptr = NULL;
-- 
2.53.0

Re: [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 12:16:38PM +0200, Elia Pinto wrote:
> diff --git a/diff.c b/diff.c
> index b4ac17f8ef..9483931c05 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1959,7 +1959,7 @@ static void emit_rewrite_diff(const char *name_a,
>  
>  struct diff_words_buffer {
>  	mmfile_t text;
> -	unsigned long alloc;
> +	size_t alloc;
>  	struct diff_words_orig {
>  		const char *begin, *end;
>  	} *orig;
> @@ -2192,7 +2192,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
>  		regex_t *word_regex)
>  {
>  	int i, j;
> -	long alloc = 0;
> +	size_t alloc = 0;

This is especially offending. I double checked that we really don't make
use of a potential negative value, but all this is ever used for is
`ALLOC_GROW()`.

Patrick

[PATCH 3/3] convert: use size_t for buffer sizes

[Elia Pinto]

The conversion code receives its buffers with a size_t length, but the
helpers that scan them, gather_stats(), gather_convert_stats(),
gather_convert_stats_ascii() and count_ident(), take an "unsigned long".
On platforms where that is 32 bits wide, like Windows, they only look
at the first "size mod 2^32" bytes of a file over 4GiB. The decision
whether a file is binary, or contains CRLF line endings or "$Id$"
keywords, is then based on that prefix, and we may convert the line
endings of a file that is binary further down, or not expand keywords
that appear after the first 4GiB.

read_blob_data_from_index(), which feeds has_crlf_in_index(),
get_cached_convert_stats_ascii() and the reading of .gitattributes from
the index, gets the size of the blob as a size_t, but hands it out
through cast_size_t_to_ulong(), so on such platforms "git add" of a file
whose staged version is over 4GiB dies when core.autocrlf is in effect.

Use size_t for all of them.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 attr.c          |  2 +-
 convert.c       | 14 +++++++-------
 read-cache-ll.h |  2 +-
 read-cache.c    |  4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/attr.c b/attr.c
index 0e63f1b6de..1acb4ef430 100644
--- a/attr.c
+++ b/attr.c
@@ -793,7 +793,7 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,
 {
 	struct attr_stack *stack = NULL;
 	char *buf;
-	unsigned long size;
+	size_t size;
 	int sparse_dir_pos = -1;
 
 	if (!istate)
diff --git a/convert.c b/convert.c
index 036506842c..ca5594cab0 100644
--- a/convert.c
+++ b/convert.c
@@ -42,9 +42,9 @@ struct text_stat {
 	unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
+static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
 {
-	unsigned long i;
+	size_t i;
 
 	memset(stats, 0, sizeof(*stats));
 
@@ -102,7 +102,7 @@ static int convert_is_binary(const struct text_stat *stats)
 	return 0;
 }
 
-static unsigned int gather_convert_stats(const char *data, unsigned long size)
+static unsigned int gather_convert_stats(const char *data, size_t size)
 {
 	struct text_stat stats;
 	int ret = 0;
@@ -119,7 +119,7 @@ static unsigned int gather_convert_stats(const char *data, unsigned long size)
 	return ret;
 }
 
-static const char *gather_convert_stats_ascii(const char *data, unsigned long size)
+static const char *gather_convert_stats_ascii(const char *data, size_t size)
 {
 	unsigned int convert_stats = gather_convert_stats(data, size);
 
@@ -141,7 +141,7 @@ const char *get_cached_convert_stats_ascii(struct index_state *istate,
 					   const char *path)
 {
 	const char *ret;
-	unsigned long sz;
+	size_t sz;
 	void *data = read_blob_data_from_index(istate, path, &sz);
 	ret = gather_convert_stats_ascii(data, sz);
 	free(data);
@@ -223,7 +223,7 @@ static void check_global_conv_flags_eol(const char *path,
 
 static int has_crlf_in_index(struct index_state *istate, const char *path)
 {
-	unsigned long sz;
+	size_t sz;
 	void *data;
 	const char *crp;
 	int has_crlf = 0;
@@ -1077,7 +1077,7 @@ static int read_convert_config(const char *var, const char *value,
 	return 0;
 }
 
-static int count_ident(const char *cp, unsigned long size)
+static int count_ident(const char *cp, size_t size)
 {
 	/*
 	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 066dd8bc3b..d279059b5f 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -416,7 +416,7 @@ int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
 void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 int index_name_is_other(struct index_state *, const char *, int);
-void *read_blob_data_from_index(struct index_state *, const char *, unsigned long *);
+void *read_blob_data_from_index(struct index_state *, const char *, size_t *);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index c4cf08a3a3..635dad1ba3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3472,7 +3472,7 @@ int index_name_is_other(struct index_state *istate, const char *name,
 }
 
 void *read_blob_data_from_index(struct index_state *istate,
-				const char *path, unsigned long *size)
+				const char *path, size_t *size)
 {
 	int pos, len;
 	size_t sz;
@@ -3503,7 +3503,7 @@ void *read_blob_data_from_index(struct index_state *istate,
 		return NULL;
 	}
 	if (size)
-		*size = cast_size_t_to_ulong(sz);
+		*size = sz;
 	return data;
 }
 
-- 
2.53.0

Re: [PATCH 3/3] convert: use size_t for buffer sizes

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 12:16:39PM +0200, Elia Pinto wrote:
> diff --git a/convert.c b/convert.c
> index 036506842c..ca5594cab0 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -42,9 +42,9 @@ struct text_stat {
>  	unsigned printable, nonprintable;
>  };
>  
> -static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
> +static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
>  {
> -	unsigned long i;
> +	size_t i;
>  
>  	memset(stats, 0, sizeof(*stats));
>  

One thing to note here is that `struct text_stat` still uses `unsigned`
counters. Is this something we should fix, as well?

> @@ -1077,7 +1077,7 @@ static int read_convert_config(const char *var, const char *value,
>  	return 0;
>  }
>  
> -static int count_ident(const char *cp, unsigned long size)
> +static int count_ident(const char *cp, size_t size)
>  {
>  	/*
>  	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"

This function is counting `$Id$` markers, so it's rather unlikely that
it ever overflows. Theoretically it's possible though.

Patrick

[PATCH v2 0/4] use size_t for a few more object and buffer sizes

[Elia Pinto]

On 64-bit Windows "long" is still 32 bits wide, so a size_t that goes
through an "unsigned long" loses anything above 4 GiB. Like v1, this
series widens three such places: the length taken by the object
write API (patch 1), the allocation size of the word-diff buffers
(patch 2) and the buffer sizes in convert.c (patch 3). Patch 4 is
new. The cover letter of v1 explains how these were found:

  https://lore.kernel.org/git/20261021101640.329990-1-gitter.spiros@gmail.com/

Changes since v1, all following Patrick's review (thanks!):

 - Patch 1 adds the test Patrick asked for. The existing "hash
   filtered files over 4GB correctly" test in t1007 only hashes; the
   new one writes the object with "git hash-object -w" and checks its
   object ID and size. It is EXPENSIVE and SIZE_T_IS_64BIT, like its
   neighbours. Truncating the length to 32 bits in
   odb_write_object_ext() on Linux, to mimic Windows, makes it fail.

 - Patch 4 is new and answers Patrick's question on patch 3: struct
   text_stat still counted in "unsigned", and count_ident() in an
   "int". Note that this one is not specific to LLP64, as "unsigned"
   is 32 bits everywhere; it is in this series because it came up
   in the review and touches the same code. A wrapped counter only
   leads to a wrong decision (e.g. no CRLF conversion for a file with
   exactly 2^32 lone LFs), not to a memory safety problem, because
   the counts are only size hints for strbuf_grow().

 - Rebased on current master; no conflicts.

Elia Pinto (4):
  odb: use size_t for the length of objects to write
  diff: use size_t for the allocation size of word-diff buffers
  convert: use size_t for buffer sizes
  convert: use size_t for text statistics and ident counts

 attr.c                 |  2 +-
 convert.c              | 24 ++++++++++++------------
 diff.c                 |  4 ++--
 odb.c                  |  2 +-
 odb.h                  |  4 ++--
 odb/source-inmemory.c  |  2 +-
 odb/source-loose.c     |  2 +-
 odb/source.h           |  2 +-
 read-cache-ll.h        |  2 +-
 read-cache.c           |  4 ++--
 t/t1007-hash-object.sh | 13 +++++++++++++
 11 files changed, 37 insertions(+), 24 deletions(-)

Range-diff against v1:
1:  0fff9532f9 ! 1:  8fcb9f10f1 odb: use size_t for the length of objects to write
    @@ Commit message
         source.
     
         The existing "hash filtered files over 4GB correctly" test in t1007
    -    only hashes. Running the same with "git hash-object -w" on Windows
    -    shows the problem, but needs another pass over a 5GiB file, so no new
    -    test is added here.
    +    only hashes, so add a variant that writes the object with "git
    +    hash-object -w" and checks both its object ID and its size. Like the
    +    other tests over 4GB, it needs EXPENSIVE and SIZE_T_IS_64BIT. Truncating
    +    the length to 32 bits in odb_write_object_ext(), as happens on Windows
    +    without this patch, makes it fail with a different object ID.
     
         Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
     
    @@ odb/source.h: static inline int odb_source_freshen_object(struct odb_source *sou
      					  enum object_type type,
      					  const struct object_id *oid,
      					  const struct object_id *compat_oid,
    +
    + ## t/t1007-hash-object.sh ##
    +@@ t/t1007-hash-object.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
    + 	test_cmp expect actual
    + '
    + 
    ++test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
    ++		'write filtered files over 4GB correctly' '
    ++	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
    ++	test_oid large5GB >expect &&
    ++	test_config filter.null-filter.clean "cat" &&
    ++	echo "big filter=null-filter" >.gitattributes &&
    ++	git hash-object -w -- big >actual &&
    ++	test_cmp expect actual &&
    ++	echo $((5*1024*1024*1024)) >expect &&
    ++	git cat-file -s $(cat actual) >actual &&
    ++	test_cmp expect actual
    ++'
    ++
    + test_done
2:  4ab9569f13 = 2:  42d4eae985 diff: use size_t for the allocation size of word-diff buffers
3:  7ea1a566b5 = 3:  88ca2ceb80 convert: use size_t for buffer sizes
-:  ---------- > 4:  84e9e38a3e convert: use size_t for text statistics and ident counts

base-commit: b9e5cff1803ab24582201ef059516eb16014b9d9
-- 
2.53.0

[PATCH v2 1/4] odb: use size_t for the length of objects to write

[Elia Pinto]

c6a4629e32 (odb: use size_t for object_info.sizep and the size APIs,
2026-06-15) widened the read side of the object database API, and the
write_object() callback of odb_source already takes a size_t. But
odb_write_object_ext(), odb_write_object() and odb_source_write_object()
in front of it, as well as write_loose_object() behind it, still take
an "unsigned long", which is only 32 bits wide on Windows.

Most callers hand them a size_t from a strbuf or from odb_read_object(),
which silently gets truncated. The most visible case is index_mem() and
index_stream_convert_blob(): when "git add" or "git hash-object -w"
runs a file over 4GiB through a conversion (core.autocrlf, which is on
by default in Git for Windows, an eol or ident attribute, or a clean
filter), we write a blob made of the first "size mod 2^32" bytes. The
index records it with the stat data of the real file, so "git status"
stays clean and nobody notices until the file is checked out again.
"git hash-object" without "-w" uses hash_object_file(), which takes a
size_t, so the two disagree on the object ID of the same file.

Use size_t throughout, including for the size kept by the in-memory
source.

The existing "hash filtered files over 4GB correctly" test in t1007
only hashes, so add a variant that writes the object with "git
hash-object -w" and checks both its object ID and its size. Like the
other tests over 4GB, it needs EXPENSIVE and SIZE_T_IS_64BIT. Truncating
the length to 32 bits in odb_write_object_ext(), as happens on Windows
without this patch, makes it fail with a different object ID.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 odb.c                  |  2 +-
 odb.h                  |  4 ++--
 odb/source-inmemory.c  |  2 +-
 odb/source-loose.c     |  2 +-
 odb/source.h           |  2 +-
 t/t1007-hash-object.sh | 13 +++++++++++++
 6 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index 2c93a9490c..1f173625b7 100644
--- a/odb.c
+++ b/odb.c
@@ -640,7 +640,7 @@ void odb_assert_oid_type(struct object_database *odb,
 }
 
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid_in,
diff --git a/odb.h b/odb.h
index dd85b880a7..45182d28cb 100644
--- a/odb.h
+++ b/odb.h
@@ -621,14 +621,14 @@ enum odb_write_object_flags {
  * Returns 0 on success, a negative error code otherwise.
  */
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid,
 			 enum odb_write_object_flags flags);
 
 static inline int odb_write_object(struct object_database *odb,
-				   const void *buf, unsigned long len,
+				   const void *buf, size_t len,
 				   enum object_type type,
 				   struct object_id *oid)
 {
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index 75cf29fa9d..c13cffc805 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -10,7 +10,7 @@
 struct inmemory_object {
 	enum object_type type;
 	const void *buf;
-	unsigned long size;
+	size_t size;
 };
 
 static const struct inmemory_object *find_cached_object(struct odb_source_inmemory *source,
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 50cb388c35..60801e70e8 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -758,7 +758,7 @@ static int end_loose_object_common(struct odb_source_loose *loose,
 
 static int write_loose_object(struct odb_source_loose *loose,
 			      const struct object_id *oid, char *hdr,
-			      int hdrlen, const void *buf, unsigned long len,
+			      int hdrlen, const void *buf, size_t len,
 			      const time_t *mtime, unsigned flags)
 {
 	int fd, ret;
diff --git a/odb/source.h b/odb/source.h
index 8b839caa39..07d5019aa5 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -451,7 +451,7 @@ static inline int odb_source_freshen_object(struct odb_source *source,
  * object ID and the compatibility object ID, if non-NULL.
  */
 static inline int odb_source_write_object(struct odb_source *source,
-					  const void *buf, unsigned long len,
+					  const void *buf, size_t len,
 					  enum object_type type,
 					  const struct object_id *oid,
 					  const struct object_id *compat_oid,
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index 463b38f990..0a89bbb455 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -297,4 +297,17 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
 	test_cmp expect actual
 '
 
+test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
+		'write filtered files over 4GB correctly' '
+	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
+	test_oid large5GB >expect &&
+	test_config filter.null-filter.clean "cat" &&
+	echo "big filter=null-filter" >.gitattributes &&
+	git hash-object -w -- big >actual &&
+	test_cmp expect actual &&
+	echo $((5*1024*1024*1024)) >expect &&
+	git cat-file -s $(cat actual) >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.53.0

Re: [PATCH v2 1/4] odb: use size_t for the length of objects to write

[Patrick Steinhardt]

On Fri, Oct 30, 2026 at 10:58:50AM +0100, Elia Pinto wrote:
> diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
> index 463b38f990..0a89bbb455 100755
> --- a/t/t1007-hash-object.sh
> +++ b/t/t1007-hash-object.sh
> @@ -297,4 +297,17 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
>  	test_cmp expect actual
>  '
>  
> +test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
> +		'write filtered files over 4GB correctly' '
> +	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
> +	test_oid large5GB >expect &&
> +	test_config filter.null-filter.clean "cat" &&
> +	echo "big filter=null-filter" >.gitattributes &&
> +	git hash-object -w -- big >actual &&
> +	test_cmp expect actual &&
> +	echo $((5*1024*1024*1024)) >expect &&

Isn't this the exact same as `big`? So...

> +	git cat-file -s $(cat actual) >actual &&

We shouldn't read from and write to the same file. Maybe write it to
"actual.size" instead?

> +	test_cmp expect actual
> +'

... can't we `test_cmp big actual` here?

Also, it might make sense to use `test_cmp_bin ()` instead.

Patrick

Re: [PATCH v2 1/4] odb: use size_t for the length of objects to write

[Elia Pinto]

On Fri, Oct 30, 2026 at 12:44:34PM +0100, Patrick Steinhardt wrote:
> > diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
> > index 463b38f990..0a89bbb455 100755
> > --- a/t/t1007-hash-object.sh
> > +++ b/t/t1007-hash-object.sh
> > @@ -297,4 +297,17 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
> >  	test_cmp expect actual
> >  '
> >  
> > +test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
> > +		'write filtered files over 4GB correctly' '
> > +	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
> > +	test_oid large5GB >expect &&
> > +	test_config filter.null-filter.clean "cat" &&
> > +	echo "big filter=null-filter" >.gitattributes &&
> > +	git hash-object -w -- big >actual &&
> > +	test_cmp expect actual &&
> > +	echo $((5*1024*1024*1024)) >expect &&
>
> Isn't this the exact same as `big`? So...

Ah, there was a slight misunderstanding here: `expect` didn't contain
the content of `big`, but its expected size in bytes (`5368709120`),
which was then compared against the output of `cat-file -s`.

That said, your point stands: checking only the size was a weak and
redundant check. The OID printed by `hash-object -w` is computed
directly from the object content that was written. If the length had
been truncated (like on Windows), the written object would be different
and its OID wouldn't match `large5GB`. I verified this by simulating the
truncation: forcing the length to 32 bits in `odb_write_object_ext()`
causes the test to fail right there on the OID comparison.

Because of this, `test_cmp big actual` isn't needed, and running it
would unnecessarily cost another expensive read of 5 GiB from the object
database.

>
> > +	git cat-file -s $(cat actual) >actual &&
>
> We shouldn't read from and write to the same file. Maybe write it to
> "actual.size" instead?

Agreed, reading from and writing to the same file was fragile, even
though it worked because shell redirection happens after expansion. In
v3 this goes away completely since we dropped the size line.

>
> Also, it might make sense to use `test_cmp_bin ()` instead.

Since the size check is gone, we only compare the OID, which is text, so
`test_cmp` remains the correct tool. `test_cmp_bin` would only make
sense for comparing binary content, which we no longer do for the
reasons mentioned above.

Instead, v3 checks that the object actually exists in the database by
running `git cat-file -e "$(cat actual)"`. This is a cheap check that
confirms `-w` wrote it successfully without needing to read back the
5 GiB content.

Thanks for the review! v3 will follow shortly in the same thread.

Elia

[PATCH v2 2/4] diff: use size_t for the allocation size of word-diff buffers

[Elia Pinto]

Since 3c7a31204f (xdiff: use size_t for buffer sizes, 2026-09-30) the
size of an mmfile_t is a size_t, but the word-diff code still grows
these buffers with ALLOC_GROW() using an "unsigned long" (in struct
diff_words_buffer) or a "long" (in diff_words_fill()) for the allocated
size.

On platforms where "long" is 32 bits wide this is not just cosmetic.
diff_words_fill() writes every word followed by a newline, so its
output can be up to twice as large as its input, which is limited by
MAX_XDIFF_SIZE to just under 1GiB. alloc_nr() computes (alloc + 16) * 3
in the type of "alloc", which overflows a signed 32-bit "long" as soon
as the buffer grows past about 700MiB. The result is undefined behavior,
in practice a negative size that REALLOC_ARRAY() turns into a huge
allocation request and a die().

The buffer in diff_words_append() is bounded by MAX_XDIFF_SIZE, so
there it is only a matter of consistency, but if that limit ever went
away, ALLOC_GROW() would truncate the requested size, and the memcpy()
right after it would overflow the buffer.

Use size_t for both.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index b4ac17f8ef..9483931c05 100644
--- a/diff.c
+++ b/diff.c
@@ -1959,7 +1959,7 @@ static void emit_rewrite_diff(const char *name_a,
 
 struct diff_words_buffer {
 	mmfile_t text;
-	unsigned long alloc;
+	size_t alloc;
 	struct diff_words_orig {
 		const char *begin, *end;
 	} *orig;
@@ -2192,7 +2192,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
 		regex_t *word_regex)
 {
 	int i, j;
-	long alloc = 0;
+	size_t alloc = 0;
 
 	out->size = 0;
 	out->ptr = NULL;
-- 
2.53.0

[PATCH v2 3/4] convert: use size_t for buffer sizes

[Elia Pinto]

The conversion code receives its buffers with a size_t length, but the
helpers that scan them, gather_stats(), gather_convert_stats(),
gather_convert_stats_ascii() and count_ident(), take an "unsigned long".
On platforms where that is 32 bits wide, like Windows, they only look
at the first "size mod 2^32" bytes of a file over 4GiB. The decision
whether a file is binary, or contains CRLF line endings or "$Id$"
keywords, is then based on that prefix, and we may convert the line
endings of a file that is binary further down, or not expand keywords
that appear after the first 4GiB.

read_blob_data_from_index(), which feeds has_crlf_in_index(),
get_cached_convert_stats_ascii() and the reading of .gitattributes from
the index, gets the size of the blob as a size_t, but hands it out
through cast_size_t_to_ulong(), so on such platforms "git add" of a file
whose staged version is over 4GiB dies when core.autocrlf is in effect.

Use size_t for all of them.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 attr.c          |  2 +-
 convert.c       | 14 +++++++-------
 read-cache-ll.h |  2 +-
 read-cache.c    |  4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/attr.c b/attr.c
index 0e63f1b6de..1acb4ef430 100644
--- a/attr.c
+++ b/attr.c
@@ -793,7 +793,7 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,
 {
 	struct attr_stack *stack = NULL;
 	char *buf;
-	unsigned long size;
+	size_t size;
 	int sparse_dir_pos = -1;
 
 	if (!istate)
diff --git a/convert.c b/convert.c
index 036506842c..ca5594cab0 100644
--- a/convert.c
+++ b/convert.c
@@ -42,9 +42,9 @@ struct text_stat {
 	unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
+static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
 {
-	unsigned long i;
+	size_t i;
 
 	memset(stats, 0, sizeof(*stats));
 
@@ -102,7 +102,7 @@ static int convert_is_binary(const struct text_stat *stats)
 	return 0;
 }
 
-static unsigned int gather_convert_stats(const char *data, unsigned long size)
+static unsigned int gather_convert_stats(const char *data, size_t size)
 {
 	struct text_stat stats;
 	int ret = 0;
@@ -119,7 +119,7 @@ static unsigned int gather_convert_stats(const char *data, unsigned long size)
 	return ret;
 }
 
-static const char *gather_convert_stats_ascii(const char *data, unsigned long size)
+static const char *gather_convert_stats_ascii(const char *data, size_t size)
 {
 	unsigned int convert_stats = gather_convert_stats(data, size);
 
@@ -141,7 +141,7 @@ const char *get_cached_convert_stats_ascii(struct index_state *istate,
 					   const char *path)
 {
 	const char *ret;
-	unsigned long sz;
+	size_t sz;
 	void *data = read_blob_data_from_index(istate, path, &sz);
 	ret = gather_convert_stats_ascii(data, sz);
 	free(data);
@@ -223,7 +223,7 @@ static void check_global_conv_flags_eol(const char *path,
 
 static int has_crlf_in_index(struct index_state *istate, const char *path)
 {
-	unsigned long sz;
+	size_t sz;
 	void *data;
 	const char *crp;
 	int has_crlf = 0;
@@ -1077,7 +1077,7 @@ static int read_convert_config(const char *var, const char *value,
 	return 0;
 }
 
-static int count_ident(const char *cp, unsigned long size)
+static int count_ident(const char *cp, size_t size)
 {
 	/*
 	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 066dd8bc3b..d279059b5f 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -416,7 +416,7 @@ int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
 void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 int index_name_is_other(struct index_state *, const char *, int);
-void *read_blob_data_from_index(struct index_state *, const char *, unsigned long *);
+void *read_blob_data_from_index(struct index_state *, const char *, size_t *);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index c4cf08a3a3..635dad1ba3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3472,7 +3472,7 @@ int index_name_is_other(struct index_state *istate, const char *name,
 }
 
 void *read_blob_data_from_index(struct index_state *istate,
-				const char *path, unsigned long *size)
+				const char *path, size_t *size)
 {
 	int pos, len;
 	size_t sz;
@@ -3503,7 +3503,7 @@ void *read_blob_data_from_index(struct index_state *istate,
 		return NULL;
 	}
 	if (size)
-		*size = cast_size_t_to_ulong(sz);
+		*size = sz;
 	return data;
 }
 
-- 
2.53.0

[PATCH v2 4/4] convert: use size_t for text statistics and ident counts

[Elia Pinto]

gather_stats() counts NULs, CRs, LFs, CRLFs and (non)printable
characters in "unsigned" fields of struct text_stat, and
count_ident() counts "$Id$" markers in an "int". Now that the buffer
sizes they work on are size_t, a large enough buffer overflows them.
Unlike the previous patches, this is not specific to LLP64: "unsigned"
is 32 bits wide on all the platforms we care about.

The counters only drive decisions, but a wrapped counter makes the
wrong one. For example, a file with exactly 2^32 lone LFs gets
lonelf == 0, so will_convert_lf_to_crlf() decides there is nothing
to convert, and a wrapped "printable" or "nonprintable" can fool the
heuristic in convert_is_binary(). In count_ident(), going past
INT_MAX is undefined behaviour.

This is not a memory safety problem: the counts are only used as
hints to strbuf_grow() in crlf_to_worktree() and ident_to_worktree(),
and the buffers are then filled with strbuf_add(), which grows them
as needed.

Use size_t for all of them, and compute the size hint in
ident_to_worktree() with st_add() and st_mult(), so that it dies
instead of wrapping around.

Triggering any of this needs more than 4GiB of input, so no test is
added.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 convert.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index ca5594cab0..34fb4ce685 100644
--- a/convert.c
+++ b/convert.c
@@ -36,10 +36,10 @@
 
 struct text_stat {
 	/* NUL, CR, LF and CRLF counts */
-	unsigned nul, lonecr, lonelf, crlf;
+	size_t nul, lonecr, lonelf, crlf;
 
 	/* These are just approximations! */
-	unsigned printable, nonprintable;
+	size_t printable, nonprintable;
 };
 
 static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
@@ -1077,12 +1077,12 @@ static int read_convert_config(const char *var, const char *value,
 	return 0;
 }
 
-static int count_ident(const char *cp, size_t size)
+static size_t count_ident(const char *cp, size_t size)
 {
 	/*
 	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"
 	 */
-	int cnt = 0;
+	size_t cnt = 0;
 	char ch;
 
 	while (size) {
@@ -1170,7 +1170,7 @@ static int ident_to_worktree(const char *src, size_t len,
 	struct object_id oid;
 	char *to_free = NULL;
 	const char *dollar, *spc;
-	int cnt;
+	size_t cnt;
 
 	if (!ident)
 		return 0;
@@ -1184,7 +1184,7 @@ static int ident_to_worktree(const char *src, size_t len,
 		to_free = strbuf_detach(buf, NULL);
 	hash_object_file(the_hash_algo, src, len, OBJ_BLOB, &oid);
 
-	strbuf_grow(buf, len + cnt * (the_hash_algo->hexsz + 3));
+	strbuf_grow(buf, st_add(len, st_mult(cnt, the_hash_algo->hexsz + 3)));
 	for (;;) {
 		/* step 1: run to the next '$' */
 		dollar = memchr(src, '$', len);
-- 
2.53.0

Re: [PATCH v2 4/4] convert: use size_t for text statistics and ident counts

[Patrick Steinhardt]

On Fri, Oct 30, 2026 at 10:58:53AM +0100, Elia Pinto wrote:
> gather_stats() counts NULs, CRs, LFs, CRLFs and (non)printable
> characters in "unsigned" fields of struct text_stat, and
> count_ident() counts "$Id$" markers in an "int". Now that the buffer
> sizes they work on are size_t, a large enough buffer overflows them.

This sounds a bit too deterministic: it doesn't necessarily overflow
them, but the possibility exists in theory.

> Unlike the previous patches, this is not specific to LLP64: "unsigned"
> is 32 bits wide on all the platforms we care about.
> 
> The counters only drive decisions, but a wrapped counter makes the
> wrong one. For example, a file with exactly 2^32 lone LFs gets
> lonelf == 0, so will_convert_lf_to_crlf() decides there is nothing
> to convert, and a wrapped "printable" or "nonprintable" can fool the
> heuristic in convert_is_binary(). In count_ident(), going past
> INT_MAX is undefined behaviour.
> 
> This is not a memory safety problem: the counts are only used as
> hints to strbuf_grow() in crlf_to_worktree() and ident_to_worktree(),
> and the buffers are then filled with strbuf_add(), which grows them
> as needed.

These paragraphs very much read like AI to me. I would appreciate a
human touch, also to demonstrate that a human was involved between us
and the AI that tries to understand what was done.

> Use size_t for all of them, and compute the size hint in
> ident_to_worktree() with st_add() and st_mult(), so that it dies
> instead of wrapping around.

That's awfully specific.

The patch itself looks good to me, thanks.

Patrick

Re: [PATCH v2 4/4] convert: use size_t for text statistics and ident counts

[Elia Pinto]

On Fri, Oct 30, 2026 at 12:44:40PM +0100, Patrick Steinhardt wrote:
> > gather_stats() counts NULs, CRs, LFs, CRLFs and (non)printable
> > characters in "unsigned" fields of struct text_stat, and
> > count_ident() counts "$Id$" markers in an "int". Now that the buffer
> > sizes they work on are size_t, a large enough buffer overflows them.
>
> This sounds a bit too deterministic: it doesn't necessarily overflow
> them, but the possibility exists in theory.

Indeed, it's a theoretical possibility rather than something that
automatically happens. In v3 I've adjusted it to say "large inputs can
overflow these counters".

>
> > Unlike the previous patches, this is not specific to LLP64: "unsigned"
> > [...]
> > as needed.
>
> These paragraphs very much read like AI to me. I would appreciate a
> human touch, also to demonstrate that a human was involved between us
> and the AI that tries to understand what was done.

You're completely right. I used AI tooling to help draft that commit
message, and it definitely shows. Fair cop.

To clarify my involvement: I review the code, check the cases, run the
tests, and take full responsibility for what gets submitted. But the
wording here missed the mark. For v3 I rewrote the message myself,
keeping it much shorter, focusing on a single example (lone LFs), and
dropping the heavy-handed explanations.

>
> > Use size_t for all of them, and compute the size hint in
> > ident_to_worktree() with st_add() and st_mult(), so that it dies
> > instead of wrapping around.
>
> That's awfully specific.

Agreed, it spells out the implementation details which are already
obvious from the diff. In v3 I've simplified it to just state that the
size hint is checked for overflow.

>
> The patch itself looks good to me, thanks.

Thanks for the careful review! v3 will follow shortly in the same
thread.

Elia

[PATCH v3 0/4] use size_t for a few more object and buffer sizes

[Elia Pinto]

Thanks to Patrick Steinhardt for reviewing v2.

Changes since v2:

 - Patch 1: drop the redundant file size check since the OID printed by
   `hash-object -w` already proves the content is complete, using
   `cat-file -e` just to verify the object exists. This also avoids
   reading and writing the same file unnecessarily.
 - Patch 2 & 3: unchanged.
 - Patch 4: shortened commit message following Patrick's comments.

Link to v2:
  https://lore.kernel.org/git/cover.1793352943.git.gitter.spiros@gmail.com/

Elia Pinto (4):
  odb: use size_t for the length of objects to write
  diff: use size_t for the allocation size of word-diff buffers
  convert: use size_t for buffer sizes
  convert: use size_t for text statistics and ident counts

 attr.c                 |  2 +-
 convert.c              | 24 ++++++++++++------------
 diff.c                 |  4 ++--
 odb.c                  |  2 +-
 odb.h                  |  4 ++--
 odb/source-inmemory.c  |  2 +-
 odb/source-loose.c     |  2 +-
 odb/source.h           |  2 +-
 read-cache-ll.h        |  2 +-
 read-cache.c           |  4 ++--
 t/t1007-hash-object.sh | 11 +++++++++++
 11 files changed, 35 insertions(+), 24 deletions(-)

Range-diff against v2:
1:  8fcb9f10f1 ! 1:  4961a1824a odb: use size_t for the length of objects to write
    @@ Commit message
     
         The existing "hash filtered files over 4GB correctly" test in t1007
         only hashes, so add a variant that writes the object with "git
    -    hash-object -w" and checks both its object ID and its size. Like the
    +    hash-object -w" and checks its object ID and that it exists. Like the
         other tests over 4GB, it needs EXPENSIVE and SIZE_T_IS_64BIT. Truncating
         the length to 32 bits in odb_write_object_ext(), as happens on Windows
         without this patch, makes it fail with a different object ID.
    @@ t/t1007-hash-object.sh: test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
     +	echo "big filter=null-filter" >.gitattributes &&
     +	git hash-object -w -- big >actual &&
     +	test_cmp expect actual &&
    -+	echo $((5*1024*1024*1024)) >expect &&
    -+	git cat-file -s $(cat actual) >actual &&
    -+	test_cmp expect actual
    ++	git cat-file -e "$(cat actual)"
     +'
     +
      test_done
2:  42d4eae985 = 2:  b27ef2a888 diff: use size_t for the allocation size of word-diff buffers
3:  88ca2ceb80 = 3:  1617177880 convert: use size_t for buffer sizes
4:  84e9e38a3e ! 4:  153463b531 convert: use size_t for text statistics and ident counts
    @@ Metadata
      ## Commit message ##
         convert: use size_t for text statistics and ident counts
     
    -    gather_stats() counts NULs, CRs, LFs, CRLFs and (non)printable
    -    characters in "unsigned" fields of struct text_stat, and
    -    count_ident() counts "$Id$" markers in an "int". Now that the buffer
    -    sizes they work on are size_t, a large enough buffer overflows them.
    -    Unlike the previous patches, this is not specific to LLP64: "unsigned"
    -    is 32 bits wide on all the platforms we care about.
    +    gather_stats() and count_ident() use "unsigned" and "int" counters to
    +    count characters and markers in buffers whose sizes are measured in
    +    size_t. On platforms where size_t can exceed 32 bits, large inputs can
    +    overflow these counters.
     
    -    The counters only drive decisions, but a wrapped counter makes the
    -    wrong one. For example, a file with exactly 2^32 lone LFs gets
    -    lonelf == 0, so will_convert_lf_to_crlf() decides there is nothing
    -    to convert, and a wrapped "printable" or "nonprintable" can fool the
    -    heuristic in convert_is_binary(). In count_ident(), going past
    -    INT_MAX is undefined behaviour.
    +    For example, a file with exactly 2^32 lone LFs wraps lonelf back
    +    to zero, causing will_convert_lf_to_crlf() to miss the conversion.
     
    -    This is not a memory safety problem: the counts are only used as
    -    hints to strbuf_grow() in crlf_to_worktree() and ident_to_worktree(),
    -    and the buffers are then filled with strbuf_add(), which grows them
    -    as needed.
    +    Switch the counters to size_t and check the size hint for overflow.
     
    -    Use size_t for all of them, and compute the size hint in
    -    ident_to_worktree() with st_add() and st_mult(), so that it dies
    -    instead of wrapping around.
    -
    -    Triggering any of this needs more than 4GiB of input, so no test is
    -    added.
    +    Triggering this requires processing over 4GiB of input, so no test
    +    is added.
     
         Suggested-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>

base-commit: b9e5cff1803ab24582201ef059516eb16014b9d9
-- 
2.53.0

[PATCH v3 1/4] odb: use size_t for the length of objects to write

[Elia Pinto]

c6a4629e32 (odb: use size_t for object_info.sizep and the size APIs,
2026-06-15) widened the read side of the object database API, and the
write_object() callback of odb_source already takes a size_t. But
odb_write_object_ext(), odb_write_object() and odb_source_write_object()
in front of it, as well as write_loose_object() behind it, still take
an "unsigned long", which is only 32 bits wide on Windows.

Most callers hand them a size_t from a strbuf or from odb_read_object(),
which silently gets truncated. The most visible case is index_mem() and
index_stream_convert_blob(): when "git add" or "git hash-object -w"
runs a file over 4GiB through a conversion (core.autocrlf, which is on
by default in Git for Windows, an eol or ident attribute, or a clean
filter), we write a blob made of the first "size mod 2^32" bytes. The
index records it with the stat data of the real file, so "git status"
stays clean and nobody notices until the file is checked out again.
"git hash-object" without "-w" uses hash_object_file(), which takes a
size_t, so the two disagree on the object ID of the same file.

Use size_t throughout, including for the size kept by the in-memory
source.

The existing "hash filtered files over 4GB correctly" test in t1007
only hashes, so add a variant that writes the object with "git
hash-object -w" and checks its object ID and that it exists. Like the
other tests over 4GB, it needs EXPENSIVE and SIZE_T_IS_64BIT. Truncating
the length to 32 bits in odb_write_object_ext(), as happens on Windows
without this patch, makes it fail with a different object ID.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 odb.c                  |  2 +-
 odb.h                  |  4 ++--
 odb/source-inmemory.c  |  2 +-
 odb/source-loose.c     |  2 +-
 odb/source.h           |  2 +-
 t/t1007-hash-object.sh | 11 +++++++++++
 6 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index 2c93a9490c..1f173625b7 100644
--- a/odb.c
+++ b/odb.c
@@ -640,7 +640,7 @@ void odb_assert_oid_type(struct object_database *odb,
 }
 
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid_in,
diff --git a/odb.h b/odb.h
index dd85b880a7..45182d28cb 100644
--- a/odb.h
+++ b/odb.h
@@ -621,14 +621,14 @@ enum odb_write_object_flags {
  * Returns 0 on success, a negative error code otherwise.
  */
 int odb_write_object_ext(struct object_database *odb,
-			 const void *buf, unsigned long len,
+			 const void *buf, size_t len,
 			 enum object_type type,
 			 struct object_id *oid,
 			 const struct object_id *compat_oid,
 			 enum odb_write_object_flags flags);
 
 static inline int odb_write_object(struct object_database *odb,
-				   const void *buf, unsigned long len,
+				   const void *buf, size_t len,
 				   enum object_type type,
 				   struct object_id *oid)
 {
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index 75cf29fa9d..c13cffc805 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -10,7 +10,7 @@
 struct inmemory_object {
 	enum object_type type;
 	const void *buf;
-	unsigned long size;
+	size_t size;
 };
 
 static const struct inmemory_object *find_cached_object(struct odb_source_inmemory *source,
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 50cb388c35..60801e70e8 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -758,7 +758,7 @@ static int end_loose_object_common(struct odb_source_loose *loose,
 
 static int write_loose_object(struct odb_source_loose *loose,
 			      const struct object_id *oid, char *hdr,
-			      int hdrlen, const void *buf, unsigned long len,
+			      int hdrlen, const void *buf, size_t len,
 			      const time_t *mtime, unsigned flags)
 {
 	int fd, ret;
diff --git a/odb/source.h b/odb/source.h
index 8b839caa39..07d5019aa5 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -451,7 +451,7 @@ static inline int odb_source_freshen_object(struct odb_source *source,
  * object ID and the compatibility object ID, if non-NULL.
  */
 static inline int odb_source_write_object(struct odb_source *source,
-					  const void *buf, unsigned long len,
+					  const void *buf, size_t len,
 					  enum object_type type,
 					  const struct object_id *oid,
 					  const struct object_id *compat_oid,
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index 463b38f990..e76557981d 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -297,4 +297,15 @@ test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
 	test_cmp expect actual
 '
 
+test_expect_success EXPENSIVE,SIZE_T_IS_64BIT \
+		'write filtered files over 4GB correctly' '
+	{ test -f big || test-tool genzeros $((5*1024*1024*1024)) >big; } &&
+	test_oid large5GB >expect &&
+	test_config filter.null-filter.clean "cat" &&
+	echo "big filter=null-filter" >.gitattributes &&
+	git hash-object -w -- big >actual &&
+	test_cmp expect actual &&
+	git cat-file -e "$(cat actual)"
+'
+
 test_done
-- 
2.53.0

[PATCH v3 2/4] diff: use size_t for the allocation size of word-diff buffers

[Elia Pinto]

Since 3c7a31204f (xdiff: use size_t for buffer sizes, 2026-09-30) the
size of an mmfile_t is a size_t, but the word-diff code still grows
these buffers with ALLOC_GROW() using an "unsigned long" (in struct
diff_words_buffer) or a "long" (in diff_words_fill()) for the allocated
size.

On platforms where "long" is 32 bits wide this is not just cosmetic.
diff_words_fill() writes every word followed by a newline, so its
output can be up to twice as large as its input, which is limited by
MAX_XDIFF_SIZE to just under 1GiB. alloc_nr() computes (alloc + 16) * 3
in the type of "alloc", which overflows a signed 32-bit "long" as soon
as the buffer grows past about 700MiB. The result is undefined behavior,
in practice a negative size that REALLOC_ARRAY() turns into a huge
allocation request and a die().

The buffer in diff_words_append() is bounded by MAX_XDIFF_SIZE, so
there it is only a matter of consistency, but if that limit ever went
away, ALLOC_GROW() would truncate the requested size, and the memcpy()
right after it would overflow the buffer.

Use size_t for both.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index b4ac17f8ef..9483931c05 100644
--- a/diff.c
+++ b/diff.c
@@ -1959,7 +1959,7 @@ static void emit_rewrite_diff(const char *name_a,
 
 struct diff_words_buffer {
 	mmfile_t text;
-	unsigned long alloc;
+	size_t alloc;
 	struct diff_words_orig {
 		const char *begin, *end;
 	} *orig;
@@ -2192,7 +2192,7 @@ static void diff_words_fill(struct diff_words_buffer *buffer, mmfile_t *out,
 		regex_t *word_regex)
 {
 	int i, j;
-	long alloc = 0;
+	size_t alloc = 0;
 
 	out->size = 0;
 	out->ptr = NULL;
-- 
2.53.0

[PATCH v3 3/4] convert: use size_t for buffer sizes

[Elia Pinto]

The conversion code receives its buffers with a size_t length, but the
helpers that scan them, gather_stats(), gather_convert_stats(),
gather_convert_stats_ascii() and count_ident(), take an "unsigned long".
On platforms where that is 32 bits wide, like Windows, they only look
at the first "size mod 2^32" bytes of a file over 4GiB. The decision
whether a file is binary, or contains CRLF line endings or "$Id$"
keywords, is then based on that prefix, and we may convert the line
endings of a file that is binary further down, or not expand keywords
that appear after the first 4GiB.

read_blob_data_from_index(), which feeds has_crlf_in_index(),
get_cached_convert_stats_ascii() and the reading of .gitattributes from
the index, gets the size of the blob as a size_t, but hands it out
through cast_size_t_to_ulong(), so on such platforms "git add" of a file
whose staged version is over 4GiB dies when core.autocrlf is in effect.

Use size_t for all of them.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 attr.c          |  2 +-
 convert.c       | 14 +++++++-------
 read-cache-ll.h |  2 +-
 read-cache.c    |  4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/attr.c b/attr.c
index 0e63f1b6de..1acb4ef430 100644
--- a/attr.c
+++ b/attr.c
@@ -793,7 +793,7 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,
 {
 	struct attr_stack *stack = NULL;
 	char *buf;
-	unsigned long size;
+	size_t size;
 	int sparse_dir_pos = -1;
 
 	if (!istate)
diff --git a/convert.c b/convert.c
index 036506842c..ca5594cab0 100644
--- a/convert.c
+++ b/convert.c
@@ -42,9 +42,9 @@ struct text_stat {
 	unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat *stats)
+static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
 {
-	unsigned long i;
+	size_t i;
 
 	memset(stats, 0, sizeof(*stats));
 
@@ -102,7 +102,7 @@ static int convert_is_binary(const struct text_stat *stats)
 	return 0;
 }
 
-static unsigned int gather_convert_stats(const char *data, unsigned long size)
+static unsigned int gather_convert_stats(const char *data, size_t size)
 {
 	struct text_stat stats;
 	int ret = 0;
@@ -119,7 +119,7 @@ static unsigned int gather_convert_stats(const char *data, unsigned long size)
 	return ret;
 }
 
-static const char *gather_convert_stats_ascii(const char *data, unsigned long size)
+static const char *gather_convert_stats_ascii(const char *data, size_t size)
 {
 	unsigned int convert_stats = gather_convert_stats(data, size);
 
@@ -141,7 +141,7 @@ const char *get_cached_convert_stats_ascii(struct index_state *istate,
 					   const char *path)
 {
 	const char *ret;
-	unsigned long sz;
+	size_t sz;
 	void *data = read_blob_data_from_index(istate, path, &sz);
 	ret = gather_convert_stats_ascii(data, sz);
 	free(data);
@@ -223,7 +223,7 @@ static void check_global_conv_flags_eol(const char *path,
 
 static int has_crlf_in_index(struct index_state *istate, const char *path)
 {
-	unsigned long sz;
+	size_t sz;
 	void *data;
 	const char *crp;
 	int has_crlf = 0;
@@ -1077,7 +1077,7 @@ static int read_convert_config(const char *var, const char *value,
 	return 0;
 }
 
-static int count_ident(const char *cp, unsigned long size)
+static int count_ident(const char *cp, size_t size)
 {
 	/*
 	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 066dd8bc3b..d279059b5f 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -416,7 +416,7 @@ int chmod_index_entry(struct index_state *, struct cache_entry *ce, char flip);
 int ce_same_name(const struct cache_entry *a, const struct cache_entry *b);
 void set_object_name_for_intent_to_add_entry(struct cache_entry *ce);
 int index_name_is_other(struct index_state *, const char *, int);
-void *read_blob_data_from_index(struct index_state *, const char *, unsigned long *);
+void *read_blob_data_from_index(struct index_state *, const char *, size_t *);
 
 /* do stat comparison even if CE_VALID is true */
 #define CE_MATCH_IGNORE_VALID		01
diff --git a/read-cache.c b/read-cache.c
index c4cf08a3a3..635dad1ba3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3472,7 +3472,7 @@ int index_name_is_other(struct index_state *istate, const char *name,
 }
 
 void *read_blob_data_from_index(struct index_state *istate,
-				const char *path, unsigned long *size)
+				const char *path, size_t *size)
 {
 	int pos, len;
 	size_t sz;
@@ -3503,7 +3503,7 @@ void *read_blob_data_from_index(struct index_state *istate,
 		return NULL;
 	}
 	if (size)
-		*size = cast_size_t_to_ulong(sz);
+		*size = sz;
 	return data;
 }
 
-- 
2.53.0

[PATCH v3 4/4] convert: use size_t for text statistics and ident counts

[Elia Pinto]

gather_stats() and count_ident() use "unsigned" and "int" counters to
count characters and markers in buffers whose sizes are measured in
size_t. On platforms where size_t can exceed 32 bits, large inputs can
overflow these counters.

For example, a file with exactly 2^32 lone LFs wraps lonelf back
to zero, causing will_convert_lf_to_crlf() to miss the conversion.

Switch the counters to size_t and check the size hint for overflow.

Triggering this requires processing over 4GiB of input, so no test
is added.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 convert.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/convert.c b/convert.c
index ca5594cab0..34fb4ce685 100644
--- a/convert.c
+++ b/convert.c
@@ -36,10 +36,10 @@
 
 struct text_stat {
 	/* NUL, CR, LF and CRLF counts */
-	unsigned nul, lonecr, lonelf, crlf;
+	size_t nul, lonecr, lonelf, crlf;
 
 	/* These are just approximations! */
-	unsigned printable, nonprintable;
+	size_t printable, nonprintable;
 };
 
 static void gather_stats(const char *buf, size_t size, struct text_stat *stats)
@@ -1077,12 +1077,12 @@ static int read_convert_config(const char *var, const char *value,
 	return 0;
 }
 
-static int count_ident(const char *cp, size_t size)
+static size_t count_ident(const char *cp, size_t size)
 {
 	/*
 	 * "$Id: 0000000000000000000000000000000000000000 $" <=> "$Id$"
 	 */
-	int cnt = 0;
+	size_t cnt = 0;
 	char ch;
 
 	while (size) {
@@ -1170,7 +1170,7 @@ static int ident_to_worktree(const char *src, size_t len,
 	struct object_id oid;
 	char *to_free = NULL;
 	const char *dollar, *spc;
-	int cnt;
+	size_t cnt;
 
 	if (!ident)
 		return 0;
@@ -1184,7 +1184,7 @@ static int ident_to_worktree(const char *src, size_t len,
 		to_free = strbuf_detach(buf, NULL);
 	hash_object_file(the_hash_algo, src, len, OBJ_BLOB, &oid);
 
-	strbuf_grow(buf, len + cnt * (the_hash_algo->hexsz + 3));
+	strbuf_grow(buf, st_add(len, st_mult(cnt, the_hash_algo->hexsz + 3)));
 	for (;;) {
 		/* step 1: run to the next '$' */
 		dollar = memchr(src, '$', len);
-- 
2.53.0