git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 1/3] merge-ort: fix massive leak

From
Elijah Newren <newren@gmail.com>
Date
Jan 24, 2021, 06:01 UTC
Message-ID
<20210124060112.1258291-2-newren@gmail.com>
In-Reply-To
<20210124060112.1258291-1-newren@gmail.com>

When a series of merges was performed (such as for a rebase or series of cherry-picks), only the data structures allocated by the final merge operation were being freed. The problem was that while picking out pieces of merge-ort to upstream, I previously misread a certain section of merge_start() and assumed it was associated with a later optimization. Include that section now, which ensures that if there was a previous merge operation, that we clear out result->priv and then re-use it for opt->priv, and otherwise we allocate opt->priv.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 merge-ort.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
diff --git a/merge-ort.c b/merge-ort.c
index 05c6b2e0dc..b5845ff6e9 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -3227,11 +3227,28 @@ static void merge_start(struct merge_options *opt, struct merge_result *result)
 	assert(opt->obuf.len == 0);
 
 	assert(opt->priv == NULL);
+	if (result->priv) {
+		opt->priv = result->priv;
+		result->priv = NULL;
+		/*
+		 * opt->priv non-NULL means we had results from a previous
+		 * run; do a few sanity checks that user didn't mess with
+		 * it in an obvious fashion.
+		 */
+		assert(opt->priv->call_depth == 0);
+		assert(!opt->priv->toplevel_dir ||
+		       0 == strlen(opt->priv->toplevel_dir));
+	}
 
 	/* Default to histogram diff.  Actually, just hardcode it...for now. */
 	opt->xdl_opts = DIFF_WITH_ALG(opt, HISTOGRAM_DIFF);
 
 	/* Initialization of opt->priv, our internal merge data */
+	if (opt->priv) {
+		clear_or_reinit_internal_opts(opt->priv, 1);
+		trace2_region_leave("merge", "allocate/init", opt->repo);
+		return;
+	}
 	opt->priv = xcalloc(1, sizeof(*opt->priv));
 
 	/* Initialization of various renames fields */
-- 
2.30.0.135.g7f7d4a3e17
Previous: Elijah NewrenNext: Derrick Stolee
Message 15 of 17 in “And so it begins...merge/rename performance work”
  1. 0/1 And so it begins...merge/rename performance workElijah Newren, Jan 8, 2021
  2. 1/1 merge-ort: begin performance work; instrument with trace2_region_* callsElijah Newren, Jan 8, 2021
  3. Taylor BlauJan 8, 2021
  4. Elijah NewrenJan 8, 2021
  5. Taylor BlauJan 8, 2021
  6. Elijah NewrenJan 9, 2021
  7. 0/1 And so it begins...merge/rename performance workElijah Newren, Jan 13, 2021
  8. 1/1 merge-ort: begin performance work; instrument with trace2_region_* callsElijah Newren, Jan 13, 2021
  9. Junio C HamanoJan 14, 2021
  10. Elijah NewrenJan 14, 2021
  11. 0/1 And so it begins...merge/rename performance workElijah Newren, Jan 15, 2021
  12. 1/1 merge-ort: begin performance work; instrument with trace2_region_* callsElijah Newren, Jan 15, 2021
  13. 0/3 And so it begins...merge/rename performance workElijah Newren, Jan 24, 2021
  14. 2/3 merge-ort: ignore the directory rename split conflict for nowElijah Newren, Jan 24, 2021
  15. 1/3 merge-ort: fix massive leakElijah Newren, Jan 24, 2021
  16. Derrick StoleeJan 24, 2021
  17. 3/3 merge-ort: begin performance work; instrument with trace2_region_* callsElijah Newren, Jan 24, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.