diff mbox series

[v2,13/23] diffcore-order: fix leaking buffer when parsing orderfiles

Message ID cd794220878a5bd77e864e293e011d2e300cfd9b.1727351062.git.ps@pks.im (mailing list archive)
State Accepted
Commit 6932ec81834003855486547cdc561cbbdd335389
Headers show
Series Memory leak fixes (pt.7) | expand

Commit Message

Patrick Steinhardt Sept. 26, 2024, 11:46 a.m. UTC
In `prepare_order()` we parse an orderfile and assign it to a global
array. In order to save on some allocations, we replace newlines with
NUL characters and then assign pointers into the allocated buffer to
that array. This can cause the buffer to be completely unreferenced
though in some cases, e.g. because the order file is empty or because we
had to use `xmemdupz()` to copy the lines instead of NUL-terminating
them.

Refactor the code to always `xmemdupz()` the strings. This is a bit
simpler, and it is rather unlikely that saving a handful of allocations
really matters. This allows us to release the string buffer and thus
plug the memory leak.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 diffcore-order.c      | 19 +++++++------------
 t/t4056-diff-order.sh |  1 +
 t/t4204-patch-id.sh   |  1 +
 3 files changed, 9 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/diffcore-order.c b/diffcore-order.c
index e7d20ebd2d..912513d3e6 100644
--- a/diffcore-order.c
+++ b/diffcore-order.c
@@ -14,8 +14,7 @@  static void prepare_order(const char *orderfile)
 {
 	int cnt, pass;
 	struct strbuf sb = STRBUF_INIT;
-	void *map;
-	char *cp, *endp;
+	const char *cp, *endp;
 	ssize_t sz;
 
 	if (order)
@@ -24,14 +23,13 @@  static void prepare_order(const char *orderfile)
 	sz = strbuf_read_file(&sb, orderfile, 0);
 	if (sz < 0)
 		die_errno(_("failed to read orderfile '%s'"), orderfile);
-	map = strbuf_detach(&sb, NULL);
-	endp = (char *) map + sz;
+	endp = sb.buf + sz;
 
 	for (pass = 0; pass < 2; pass++) {
 		cnt = 0;
-		cp = map;
+		cp = sb.buf;
 		while (cp < endp) {
-			char *ep;
+			const char *ep;
 			for (ep = cp; ep < endp && *ep != '\n'; ep++)
 				;
 			/* cp to ep has one line */
@@ -40,12 +38,7 @@  static void prepare_order(const char *orderfile)
 			else if (pass == 0)
 				cnt++;
 			else {
-				if (*ep == '\n') {
-					*ep = 0;
-					order[cnt] = cp;
-				} else {
-					order[cnt] = xmemdupz(cp, ep - cp);
-				}
+				order[cnt] = xmemdupz(cp, ep - cp);
 				cnt++;
 			}
 			if (ep < endp)
@@ -57,6 +50,8 @@  static void prepare_order(const char *orderfile)
 			ALLOC_ARRAY(order, cnt);
 		}
 	}
+
+	strbuf_release(&sb);
 }
 
 static int match_order(const char *path)
diff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh
index aec1d9d1b4..32c5fcb9a2 100755
--- a/t/t4056-diff-order.sh
+++ b/t/t4056-diff-order.sh
@@ -5,6 +5,7 @@  test_description='diff order & rotate'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 create_files () {
diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh
index dc8ddb10af..c0a4a02dcf 100755
--- a/t/t4204-patch-id.sh
+++ b/t/t4204-patch-id.sh
@@ -5,6 +5,7 @@  test_description='git patch-id'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup' '