[PATCH v2 2/2] Fix minor memory leak found by LeakSanitizer.

From

KGKevin Backhouse via GitGitGadget <gitgitgadget@gmail.com>

Date

Aug 24, 2023, 14:12 UTC

Message-ID

<353e1960b4466912c508733e5a7eb80884cd9bdd.1692886365.git.gitgitgadget@gmail.com>

In-Reply-To

<pull.1577.v2.git.1692886365.gitgitgadget@gmail.com>

From: Kevin Backhouse <kevinbackhouse@github.com>

Fix this by freeing the list in the code paths for error returns.

Signed-off-by: Kevin Backhouse <kevinbackhouse@github.com>
---
 merge-ort-wrappers.c |  4 +++-
 merge-ort.c          |  4 +++-
 merge-recursive.c    | 32 ++++++++++++++++++++++----------
 3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c
index 4acedf3c338..aeb56c9970c 100644
--- a/merge-ort-wrappers.c
+++ b/merge-ort-wrappers.c
@@ -54,8 +54,10 @@ int merge_ort_recursive(struct merge_options *opt,
 	struct tree *head = repo_get_commit_tree(opt->repo, side1);
 	struct merge_result tmp;
 
-	if (unclean(opt, head))
+	if (unclean(opt, head)) {
+		free_commit_list(merge_bases);
 		return -1;
+	}
 
 	memset(&tmp, 0, sizeof(tmp));
 	merge_incore_recursive(opt, merge_bases, side1, side2, &tmp);
diff --git a/merge-ort.c b/merge-ort.c
index 8631c997002..a0eb91fb011 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -5070,8 +5070,10 @@ static void merge_ort_internal(struct merge_options *opt,
 		opt->branch1 = "Temporary merge branch 1";
 		opt->branch2 = "Temporary merge branch 2";
 		merge_ort_internal(opt, NULL, prev, next, result);
-		if (result->clean < 0)
+		if (result->clean < 0) {
+			free_commit_list(merge_bases);
 			return;
+		}
 		opt->branch1 = saved_b1;
 		opt->branch2 = saved_b2;
 		opt->priv->call_depth--;
diff --git a/merge-recursive.c b/merge-recursive.c
index 6a4081bb0f5..49e54d3722f 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3652,14 +3652,18 @@ static int merge_recursive_internal(struct merge_options *opt,
 		opt->branch1 = "Temporary merge branch 1";
 		opt->branch2 = "Temporary merge branch 2";
 		if (merge_recursive_internal(opt, merged_merge_bases, iter->item,
-					     NULL, &merged_merge_bases) < 0)
-			return -1;
+					     NULL, &merged_merge_bases) < 0) {
+			clean = -1;
+			goto out;
+		}
 		opt->branch1 = saved_b1;
 		opt->branch2 = saved_b2;
 		opt->priv->call_depth--;
 
-		if (!merged_merge_bases)
-			return err(opt, _("merge returned no commit"));
+		if (!merged_merge_bases) {
+			clean = err(opt, _("merge returned no commit"));
+			goto out;
+		}
 	}
 
 	/*
@@ -3682,8 +3686,11 @@ static int merge_recursive_internal(struct merge_options *opt,
 				     repo_get_commit_tree(opt->repo,
 							  merged_merge_bases),
 				     &result_tree);
+
+out:
 	strbuf_release(&merge_base_abbrev);
 	opt->ancestor = NULL;  /* avoid accidental re-use of opt->ancestor */
+	free_commit_list(merge_bases);
 	if (clean < 0) {
 		flush_output(opt);
 		return clean;
@@ -3729,6 +3736,9 @@ static int merge_start(struct merge_options *opt, struct tree *head)
 	assert(!opt->record_conflict_msgs_as_headers);
 	assert(!opt->msg_header_prefix);
 
+	CALLOC_ARRAY(opt->priv, 1);
+	string_list_init_dup(&opt->priv->df_conflict_file_set);
+
 	/* Sanity check on repo state; index must match head */
 	if (repo_index_has_changes(opt->repo, head, &sb)) {
 		err(opt, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
@@ -3737,16 +3747,13 @@ static int merge_start(struct merge_options *opt, struct tree *head)
 		return -1;
 	}
 
-	CALLOC_ARRAY(opt->priv, 1);
-	string_list_init_dup(&opt->priv->df_conflict_file_set);
 	return 0;
 }
 
 static void merge_finalize(struct merge_options *opt)
 {
 	flush_output(opt);
-	if (!opt->priv->call_depth && opt->buffer_output < 2)
-		strbuf_release(&opt->obuf);
+	strbuf_release(&opt->obuf);
 	if (show(opt, 2))
 		diff_warn_rename_limit("merge.renamelimit",
 				       opt->priv->needed_rename_limit, 0);
@@ -3763,8 +3770,10 @@ int merge_trees(struct merge_options *opt,
 
 	assert(opt->ancestor != NULL);
 
-	if (merge_start(opt, head))
+	if (merge_start(opt, head)) {
+		merge_finalize(opt);
 		return -1;
+	}
 	clean = merge_trees_internal(opt, head, merge, merge_base, &ignored);
 	merge_finalize(opt);
 
@@ -3785,8 +3794,11 @@ int merge_recursive(struct merge_options *opt,
 	prepare_repo_settings(opt->repo);
 	opt->repo->settings.command_requires_full_index = 1;
 
-	if (merge_start(opt, repo_get_commit_tree(opt->repo, h1)))
+	if (merge_start(opt, repo_get_commit_tree(opt->repo, h1))) {
+		free_commit_list(merge_bases);
+		merge_finalize(opt);
 		return -1;
+	}
 	clean = merge_recursive_internal(opt, h1, h2, merge_bases, result);
 	merge_finalize(opt);
 

-- 
gitgitgadget