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

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

[Elia Pinto]

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

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

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

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

Elia Pinto (3):
  csum-file: use size_t for the count passed to hashwrite()
  diff: use size_t for the size passed to count_lines()
  diffcore-delta: use size_t for the size of the buffer to hash

 csum-file.c      | 2 +-
 csum-file.h      | 2 +-
 diff.c           | 2 +-
 diffcore-delta.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)


base-commit: 50fe5cc63f9cd6e89b87a2c9ccf96a444f4cfdb7
-- 
2.53.0

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

[Elia Pinto]

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

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

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

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

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

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

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

[Elia Pinto]

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

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

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

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

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

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

diff --git a/diff.c b/diff.c
index b4ac17f8ef..ed7a931c41 100644
--- a/diff.c
+++ b/diff.c
@@ -655,7 +655,7 @@ struct line_range_callback {
 	int ret;			/* latched error from orig_line_fn */
 };
 
-static int count_lines(const char *data, int size)
+static int count_lines(const char *data, size_t size)
 {
 	int count, ch, completely_empty = 1, nl_just_seen = 0;
 	count = 0;
-- 
2.53.0

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

[Elia Pinto]

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

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

Make the size variable "size_t".

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

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 diffcore-delta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/diffcore-delta.c b/diffcore-delta.c
index 2b7db39983..7f10f79ffe 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -127,7 +127,7 @@ static struct spanhash_top *hash_chars(struct repository *r,
 	unsigned int accum1, accum2, hashval;
 	struct spanhash_top *hash;
 	unsigned char *buf = one->data;
-	unsigned int sz = one->size;
+	size_t sz = one->size;
 	int is_text = !diff_filespec_is_binary(r, one);
 
 	i = INITIAL_HASH_SIZE;
-- 
2.53.0