[PATCH 2/3] test-delta: use strbufs to hold input files

From

Jeff King <peff@peff.net>

Date

Jul 24, 2025, 00:02 UTC

Message-ID

<20250724000211.GB597294@coredump.intra.peff.net>

In-Reply-To

<20250723235929.GB592873@coredump.intra.peff.net>

This incidentally fixes two small bugs:

  1. We stat() the files and allocate our buffers based on st.st_size.
     But that is an off_t which may be larger than the size_t we'd use
     to allocate. We should use xsize_t() to do a checked conversion.
     Otherwise integer truncation (on a file >4GB) could cause us to
     under-allocate (though in practice this does not result in a buffer
     overflow because the same truncation happens when read_in_full()
     also takes a size_t).

  2. We get the size from st.st_size, and then try to read_in_full()
     that many bytes. But it may return fewer bytes than expected (if
     the file changed racily and we get an early EOF), leading us to
     read uninitialized bytes in the allocated buffer. We don't notice
     because we only check the value for error, not that we got the
     expected number of bytes.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-delta.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/t/helper/test-delta.c b/t/helper/test-delta.c
index 4495b32b49..7945793078 100644
--- a/t/helper/test-delta.c
+++ b/t/helper/test-delta.c
@@ -11,45 +11,33 @@
 #include "test-tool.h"
 #include "git-compat-util.h"
 #include "delta.h"
+#include "strbuf.h"
 
 static const char usage_str[] =
 	"test-tool delta (-d|-p) <from_file> <data_file> <out_file>";
 
 int cmd__delta(int argc, const char **argv)
 {
 	int fd;
-	struct stat st;
-	void *from_buf = NULL, *data_buf = NULL, *out_buf = NULL;
-	unsigned long from_size, data_size, out_size;
+	struct strbuf from = STRBUF_INIT, data = STRBUF_INIT;
+	char *out_buf;
+	unsigned long out_size;
 
 	if (argc != 5 || (strcmp(argv[1], "-d") && strcmp(argv[1], "-p")))
 		usage(usage_str);
 
-	fd = xopen(argv[2], O_RDONLY);
-	if (fstat(fd, &st) < 0)
-		die_errno("fstat(%s)", argv[2]);
-	from_size = st.st_size;
-	from_buf = xmalloc(from_size);
-	if (read_in_full(fd, from_buf, from_size) < 0)
-		die_errno("read(%s)", argv[2]);
-	close(fd);
-
-	fd = xopen(argv[3], O_RDONLY);
-	if (fstat(fd, &st) < 0)
-		die_errno("fstat(%s)", argv[3]);
-	data_size = st.st_size;
-	data_buf = xmalloc(data_size);
-	if (read_in_full(fd, data_buf, data_size) < 0)
-		die_errno("read(%s)", argv[3]);
-	close(fd);
+	if (strbuf_read_file(&from, argv[2], 0) < 0)
+		die_errno("unable to read '%s'", argv[2]);
+	if (strbuf_read_file(&data, argv[3], 0) < 0)
+		die_errno("unable to read '%s'", argv[3]);
 
 	if (argv[1][1] == 'd')
-		out_buf = diff_delta(from_buf, from_size,
-				     data_buf, data_size,
+		out_buf = diff_delta(from.buf, from.len,
+				     data.buf, data.len,
 				     &out_size, 0);
 	else
-		out_buf = patch_delta(from_buf, from_size,
-				      data_buf, data_size,
+		out_buf = patch_delta(from.buf, from.len,
+				      data.buf, data.len,
 				      &out_size);
 	if (!out_buf)
 		die("delta operation failed (returned NULL)");
@@ -58,8 +46,8 @@ int cmd__delta(int argc, const char **argv)
 	if (write_in_full(fd, out_buf, out_size) < 0)
 		die_errno("write(%s)", argv[4]);
 
-	free(from_buf);
-	free(data_buf);
+	strbuf_release(&from);
+	strbuf_release(&data);
 	free(out_buf);
 
 	return 0;

-- 
2.50.1.666.gdb1e186d6a