* [PATCH 0/3] use size_t for a few more object and buffer sizes
@ 2026-10-21 10:16 Elia Pinto
2026-10-21 10:16 ` [PATCH 1/3] odb: use size_t for the length of objects to write Elia Pinto
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Elia Pinto @ 2026-10-21 10:16 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Jeff King, 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] odb: use size_t for the length of objects to write 2026-10-21 10:16 [PATCH 0/3] use size_t for a few more object and buffer sizes Elia Pinto @ 2026-10-21 10:16 ` Elia Pinto 2026-10-21 11:24 ` Patrick Steinhardt 2026-10-21 10:16 ` [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers Elia Pinto ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Elia Pinto @ 2026-10-21 10:16 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano, Patrick Steinhardt 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] odb: use size_t for the length of objects to write 2026-10-21 10:16 ` [PATCH 1/3] odb: use size_t for the length of objects to write Elia Pinto @ 2026-10-21 11:24 ` Patrick Steinhardt 0 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-10-21 11:24 UTC (permalink / raw) To: Elia Pinto; +Cc: git, Johannes Schindelin, Jeff King, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers 2026-10-21 10:16 [PATCH 0/3] use size_t for a few more object and buffer sizes Elia Pinto 2026-10-21 10:16 ` [PATCH 1/3] odb: use size_t for the length of objects to write Elia Pinto @ 2026-10-21 10:16 ` Elia Pinto 2026-10-21 11:24 ` Patrick Steinhardt 2026-10-21 10:16 ` [PATCH 3/3] convert: use size_t for buffer sizes Elia Pinto 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto 3 siblings, 1 reply; 14+ messages in thread From: Elia Pinto @ 2026-10-21 10:16 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers 2026-10-21 10:16 ` [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers Elia Pinto @ 2026-10-21 11:24 ` Patrick Steinhardt 0 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-10-21 11:24 UTC (permalink / raw) To: Elia Pinto; +Cc: git, Johannes Schindelin, Jeff King, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] convert: use size_t for buffer sizes 2026-10-21 10:16 [PATCH 0/3] use size_t for a few more object and buffer sizes Elia Pinto 2026-10-21 10:16 ` [PATCH 1/3] odb: use size_t for the length of objects to write Elia Pinto 2026-10-21 10:16 ` [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers Elia Pinto @ 2026-10-21 10:16 ` Elia Pinto 2026-10-21 11:24 ` Patrick Steinhardt 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto 3 siblings, 1 reply; 14+ messages in thread From: Elia Pinto @ 2026-10-21 10:16 UTC (permalink / raw) To: git Cc: Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano, Shuqi Liang, Karthik Nayak, Toon Claes 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] convert: use size_t for buffer sizes 2026-10-21 10:16 ` [PATCH 3/3] convert: use size_t for buffer sizes Elia Pinto @ 2026-10-21 11:24 ` Patrick Steinhardt 0 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-10-21 11:24 UTC (permalink / raw) To: Elia Pinto Cc: git, Johannes Schindelin, Jeff King, Junio C Hamano, Shuqi Liang, Karthik Nayak, Toon Claes 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 0/4] use size_t for a few more object and buffer sizes 2026-10-21 10:16 [PATCH 0/3] use size_t for a few more object and buffer sizes Elia Pinto ` (2 preceding siblings ...) 2026-10-21 10:16 ` [PATCH 3/3] convert: use size_t for buffer sizes Elia Pinto @ 2026-10-30 9:58 ` Elia Pinto 2026-10-30 9:58 ` [PATCH v2 1/4] odb: use size_t for the length of objects to write Elia Pinto ` (3 more replies) 3 siblings, 4 replies; 14+ messages in thread From: Elia Pinto @ 2026-10-30 9:58 UTC (permalink / raw) To: git Cc: Patrick Steinhardt, Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano, Toon Claes, Shuqi Liang, Karthik Nayak, Ævar Arnfjörð Bjarmason 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] odb: use size_t for the length of objects to write 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto @ 2026-10-30 9:58 ` Elia Pinto 2026-10-30 11:44 ` Patrick Steinhardt 2026-10-30 9:58 ` [PATCH v2 2/4] diff: use size_t for the allocation size of word-diff buffers Elia Pinto ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Elia Pinto @ 2026-10-30 9:58 UTC (permalink / raw) To: git Cc: Patrick Steinhardt, Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/4] odb: use size_t for the length of objects to write 2026-10-30 9:58 ` [PATCH v2 1/4] odb: use size_t for the length of objects to write Elia Pinto @ 2026-10-30 11:44 ` Patrick Steinhardt 0 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-10-30 11:44 UTC (permalink / raw) To: Elia Pinto; +Cc: git, Johannes Schindelin, Jeff King, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] diff: use size_t for the allocation size of word-diff buffers 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto 2026-10-30 9:58 ` [PATCH v2 1/4] odb: use size_t for the length of objects to write Elia Pinto @ 2026-10-30 9:58 ` Elia Pinto 2026-10-30 9:58 ` [PATCH v2 3/4] convert: use size_t for buffer sizes Elia Pinto 2026-10-30 9:58 ` [PATCH v2 4/4] convert: use size_t for text statistics and ident counts Elia Pinto 3 siblings, 0 replies; 14+ messages in thread From: Elia Pinto @ 2026-10-30 9:58 UTC (permalink / raw) To: git Cc: Patrick Steinhardt, Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] convert: use size_t for buffer sizes 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto 2026-10-30 9:58 ` [PATCH v2 1/4] odb: use size_t for the length of objects to write Elia Pinto 2026-10-30 9:58 ` [PATCH v2 2/4] diff: use size_t for the allocation size of word-diff buffers Elia Pinto @ 2026-10-30 9:58 ` Elia Pinto 2026-10-30 9:58 ` [PATCH v2 4/4] convert: use size_t for text statistics and ident counts Elia Pinto 3 siblings, 0 replies; 14+ messages in thread From: Elia Pinto @ 2026-10-30 9:58 UTC (permalink / raw) To: git Cc: Patrick Steinhardt, Johannes Schindelin, Jeff King, Elia Pinto, Toon Claes, Karthik Nayak, Shuqi Liang, Junio C Hamano 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] convert: use size_t for text statistics and ident counts 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto ` (2 preceding siblings ...) 2026-10-30 9:58 ` [PATCH v2 3/4] convert: use size_t for buffer sizes Elia Pinto @ 2026-10-30 9:58 ` Elia Pinto 2026-10-30 11:44 ` Patrick Steinhardt 3 siblings, 1 reply; 14+ messages in thread From: Elia Pinto @ 2026-10-30 9:58 UTC (permalink / raw) To: git Cc: Patrick Steinhardt, Johannes Schindelin, Jeff King, Elia Pinto, Junio C Hamano, Ævar Arnfjörð Bjarmason 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 4/4] convert: use size_t for text statistics and ident counts 2026-10-30 9:58 ` [PATCH v2 4/4] convert: use size_t for text statistics and ident counts Elia Pinto @ 2026-10-30 11:44 ` Patrick Steinhardt 0 siblings, 0 replies; 14+ messages in thread From: Patrick Steinhardt @ 2026-10-30 11:44 UTC (permalink / raw) To: Elia Pinto Cc: git, Johannes Schindelin, Jeff King, Junio C Hamano, Ævar Arnfjörð Bjarmason 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-10-30 11:44 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-21 10:16 [PATCH 0/3] use size_t for a few more object and buffer sizes Elia Pinto 2026-10-21 10:16 ` [PATCH 1/3] odb: use size_t for the length of objects to write Elia Pinto 2026-10-21 11:24 ` Patrick Steinhardt 2026-10-21 10:16 ` [PATCH 2/3] diff: use size_t for the allocation size of word-diff buffers Elia Pinto 2026-10-21 11:24 ` Patrick Steinhardt 2026-10-21 10:16 ` [PATCH 3/3] convert: use size_t for buffer sizes Elia Pinto 2026-10-21 11:24 ` Patrick Steinhardt 2026-10-30 9:58 ` [PATCH v2 0/4] use size_t for a few more object and " Elia Pinto 2026-10-30 9:58 ` [PATCH v2 1/4] odb: use size_t for the length of objects to write Elia Pinto 2026-10-30 11:44 ` Patrick Steinhardt 2026-10-30 9:58 ` [PATCH v2 2/4] diff: use size_t for the allocation size of word-diff buffers Elia Pinto 2026-10-30 9:58 ` [PATCH v2 3/4] convert: use size_t for buffer sizes Elia Pinto 2026-10-30 9:58 ` [PATCH v2 4/4] convert: use size_t for text statistics and ident counts Elia Pinto 2026-10-30 11:44 ` Patrick Steinhardt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.