diff mbox series

[20/22] match-trees: fix leaking prefixes in `shift_tree()`

Message ID 05461e3b1c02488046ef480f20c109b51b9b7691.1724656120.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series Memory leak fixes (pt.6) | expand

Commit Message

Patrick Steinhardt Aug. 26, 2024, 7:22 a.m. UTC
In `shift_tree()` we allocate two empty strings that we end up
passing to `match_trees()`. If that function finds a better match it
will update these pointers to point to a newly allocated strings,
freeing the old strings. We never free the final results though, neither
the ones we have allocated ourselves, nor the one that `match_trees()`
might've returned to us.

Fix the resulting memory leaks by creating a common exit path where we
free them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 match-trees.c            | 10 +++++++---
 t/t6409-merge-subtree.sh |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Junio C Hamano Sept. 4, 2024, 10:42 p.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

> In `shift_tree()` we allocate two empty strings that we end up
> passing to `match_trees()`. If that function finds a better match it
> will update these pointers to point to a newly allocated strings,
> freeing the old strings. We never free the final results though, neither
> the ones we have allocated ourselves, nor the one that `match_trees()`
> might've returned to us.
>
> Fix the resulting memory leaks by creating a common exit path where we
> free them.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  match-trees.c            | 10 +++++++---
>  t/t6409-merge-subtree.sh |  1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)

We are not going to take the "best_match" out of this
function, so somebody ought to free it, and that somebody must be
this function.

Makes sense.
diff mbox series

Patch

diff --git a/match-trees.c b/match-trees.c
index f17c74d483f..147b03abf18 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -294,18 +294,22 @@  void shift_tree(struct repository *r,
 		unsigned short mode;
 
 		if (!*del_prefix)
-			return;
+			goto out;
 
 		if (get_tree_entry(r, hash2, del_prefix, shifted, &mode))
 			die("cannot find path %s in tree %s",
 			    del_prefix, oid_to_hex(hash2));
-		return;
+		goto out;
 	}
 
 	if (!*add_prefix)
-		return;
+		goto out;
 
 	splice_tree(hash1, add_prefix, hash2, shifted);
+
+out:
+	free(add_prefix);
+	free(del_prefix);
 }
 
 /*
diff --git a/t/t6409-merge-subtree.sh b/t/t6409-merge-subtree.sh
index e9ba6f1690d..528615b981f 100755
--- a/t/t6409-merge-subtree.sh
+++ b/t/t6409-merge-subtree.sh
@@ -5,6 +5,7 @@  test_description='subtree merge strategy'
 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 '