[PATCH 05/10] commit-graph: free all elements of graph chain

From: Jeff King <peff@peff.net>
Date: Oct 3, 2023, 20:29 UTC
Message-ID: <20231003202930.GE7812@coredump.intra.peff.net>
In-Reply-To: <20231003202504.GA7697@coredump.intra.peff.net>

When running "commit-graph verify", we call free_commit_graph(). That's sufficient for the case of a single graph file, but if we loaded a chain of split graph files, they form a linked list via the base_graph pointers. We need to free all of them, or we leak all but the first struct.

We can make this work by teaching free_commit_graph() to walk the base_graph pointers and free each element. This in turn lets us simplify close_commit_graph(), which does the same thing by recursion (we cannot just use close_commit_graph() in "commit-graph verify", as the function takes a pointer to an object store, and the verify command creates a single one-off graph struct).

While indenting the code in free_commit_graph() for the loop, I noticed that setting g->data to NULL is rather pointless, as we free the struct a few lines later. So I cleaned that up while we're here.

Signed-off-by: Jeff King <peff@peff.net>
---
 commit-graph.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index dc54ef4776..2f75ecd9ae 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -723,19 +723,10 @@ struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r)
 	return NULL;
 }
 
-static void close_commit_graph_one(struct commit_graph *g)
-{
-	if (!g)
-		return;
-
-	close_commit_graph_one(g->base_graph);
-	free_commit_graph(g);
-}
-
 void close_commit_graph(struct raw_object_store *o)
 {
 	clear_commit_graph_data_slab(&commit_graph_data_slab);
-	close_commit_graph_one(o->commit_graph);
+	free_commit_graph(o->commit_graph);
 	o->commit_graph = NULL;
 }
 
@@ -2753,15 +2744,17 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
 
 void free_commit_graph(struct commit_graph *g)
 {
-	if (!g)
-		return;
-	if (g->data) {
-		munmap((void *)g->data, g->data_len);
-		g->data = NULL;
+	while (g) {
+		struct commit_graph *next = g->base_graph;
+
+		if (g->data)
+			munmap((void *)g->data, g->data_len);
+		free(g->filename);
+		free(g->bloom_filter_settings);
+		free(g);
+
+		g = next;
 	}
-	free(g->filename);
-	free(g->bloom_filter_settings);
-	free(g);
 }
 
 void disable_commit_graph(struct repository *r)

-- 
2.42.0.810.gbc538a0ee6