[PATCH 13/23] line-log: fix several memory leaks

From

Patrick Steinhardt <ps@pks.im>

Date

Sep 30, 2024, 09:13 UTC

Message-ID

<d3121c2522cc723ab4df71979fd9194260a6a57e.1727687410.git.ps@pks.im>

In-Reply-To

<cover.1727687410.git.ps@pks.im>

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 line-log.c | 54 +++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/line-log.c b/line-log.c
index 89e0ea4562..ee48988c66 100644
--- a/line-log.c
+++ b/line-log.c
@@ -248,8 +248,10 @@ static void line_log_data_init(struct line_log_data *r)
 static void line_log_data_clear(struct line_log_data *r)
 {
 	range_set_release(&r->ranges);
+	free(r->path);
 	if (r->pair)
 		diff_free_filepair(r->pair);
+	diff_ranges_release(&r->diff);
 }
 
 static void free_line_log_data(struct line_log_data *r)
@@ -571,7 +573,8 @@ parse_lines(struct repository *r, struct commit *commit,
 	struct line_log_data *p;
 
 	for_each_string_list_item(item, args) {
-		const char *name_part, *range_part;
+		const char *name_part;
+		char *range_part;
 		char *full_name;
 		struct diff_filespec *spec;
 		long begin = 0, end = 0;
@@ -615,6 +618,7 @@ parse_lines(struct repository *r, struct commit *commit,
 
 		free_filespec(spec);
 		FREE_AND_NULL(ends);
+		free(range_part);
 	}
 
 	for (p = ranges; p; p = p->next)
@@ -760,15 +764,13 @@ static void parse_pathspec_from_ranges(struct pathspec *pathspec,
 {
 	struct line_log_data *r;
 	struct strvec array = STRVEC_INIT;
-	const char **paths;
 
 	for (r = range; r; r = r->next)
 		strvec_push(&array, r->path);
-	paths = strvec_detach(&array);
 
-	parse_pathspec(pathspec, 0, PATHSPEC_PREFER_FULL, "", paths);
-	/* strings are now owned by pathspec */
-	free(paths);
+	parse_pathspec(pathspec, 0, PATHSPEC_PREFER_FULL, "", array.v);
+
+	strvec_clear(&array);
 }
 
 void line_log_init(struct rev_info *rev, const char *prefix, struct string_list *args)
@@ -781,6 +783,8 @@ void line_log_init(struct rev_info *rev, const char *prefix, struct string_list
 	add_line_range(rev, commit, range);
 
 	parse_pathspec_from_ranges(&rev->diffopt.pathspec, range);
+
+	free_line_log_data(range);
 }
 
 static void move_diff_queue(struct diff_queue_struct *dst,
@@ -1131,10 +1135,18 @@ static int process_all_files(struct line_log_data **range_out,
 			while (rg && strcmp(rg->path, pair->two->path))
 				rg = rg->next;
 			assert(rg);
+			if (rg->pair)
+				diff_free_filepair(rg->pair);
 			rg->pair = diff_filepair_dup(queue->queue[i]);
+			diff_ranges_release(&rg->diff);
 			memcpy(&rg->diff, pairdiff, sizeof(struct diff_ranges));
+			FREE_AND_NULL(pairdiff);
+		}
+
+		if (pairdiff) {
+			diff_ranges_release(pairdiff);
+			free(pairdiff);
 		}
-		free(pairdiff);
 	}
 
 	return changed;
@@ -1212,12 +1224,13 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
 	struct commit_list *p;
 	int i;
 	int nparents = commit_list_count(commit->parents);
+	int ret;
 
 	if (nparents > 1 && rev->first_parent_only)
 		nparents = 1;
 
 	ALLOC_ARRAY(diffqueues, nparents);
-	ALLOC_ARRAY(cand, nparents);
+	CALLOC_ARRAY(cand, nparents);
 	ALLOC_ARRAY(parents, nparents);
 
 	p = commit->parents;
@@ -1229,7 +1242,6 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
 
 	for (i = 0; i < nparents; i++) {
 		int changed;
-		cand[i] = NULL;
 		changed = process_all_files(&cand[i], rev, &diffqueues[i], range);
 		if (!changed) {
 			/*
@@ -1237,13 +1249,10 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
 			 * don't follow any other path in history
 			 */
 			add_line_range(rev, parents[i], cand[i]);
-			clear_commit_line_range(rev, commit);
 			commit_list_append(parents[i], &commit->parents);
-			free(parents);
-			free(cand);
-			free_diffqueues(nparents, diffqueues);
-			/* NEEDSWORK leaking like a sieve */
-			return 0;
+
+			ret = 0;
+			goto out;
 		}
 	}
 
@@ -1251,18 +1260,25 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
 	 * No single parent took the blame.  We add the candidates
 	 * from the above loop to the parents.
 	 */
-	for (i = 0; i < nparents; i++) {
+	for (i = 0; i < nparents; i++)
 		add_line_range(rev, parents[i], cand[i]);
-	}
 
+	ret = 1;
+
+out:
 	clear_commit_line_range(rev, commit);
 	free(parents);
+	for (i = 0; i < nparents; i++) {
+		if (!cand[i])
+			continue;
+		line_log_data_clear(cand[i]);
+		free(cand[i]);
+	}
 	free(cand);
 	free_diffqueues(nparents, diffqueues);
-	return 1;
+	return ret;
 
 	/* NEEDSWORK evil merge detection stuff */
-	/* NEEDSWORK leaking like a sieve */
 }
 
 int line_log_process_ranges_arbitrary_commit(struct rev_info *rev, struct commit *commit)

-- 
2.46.2.852.g229c0bf0e5.dirty