Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Taylor Blau <hidden>
Date: 2026-07-07 13:49:26
On Tue, Jul 07, 2026 at 09:59:43AM +0000, Kristofer Karlsson via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
diff --git a/commit-graph.c b/commit-graph.c index 4e39a048c4..c2a711cceb 100644 --- a/commit-graph.c +++ b/commit-graph.c@@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source, g = prepare_commit_graph(ctx.r); for (struct commit_graph *chain = g; chain; chain = chain->base_graph) - g->topo_levels = &topo_levels; + chain->topo_levels = &topo_levels; if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS) ctx.changed_paths = 1;
Looks obviously good. I think that there is a more permanent fix, though, which would have not allowed this bug to evade both its author, and reviewer (me). I *think* that we may clear up some scoping issues if we removed g->topo_levels entirely, and instead stored it in the write_commit_graph_ctx struct. I haven't thought through the implications of doing so completely, so it's entirely possible that this idea is bunk for some other reason. But it was the first thing that came to mind, and so feels worth exploring to see if it might have prevented something like this from ever happening in the first place. Thanks, Taylor