test-delta: use strbufs to hold input files

We want to read the whole contents of two files into memory. If we
switch from raw ptr/len pairs to strbufs, we can use strbuf_read_file()
to shorten the code.

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.

The strbuf code doesn't run into this, because it just reads to EOF,
expanding the buffer dynamically as necessary. Neither bug is a big deal
for a test helper, but fixing them is a nice bonus on top of simplifying
the code.

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: peff

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;