| Message ID | 20231003203055.GH7812@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 274bfa7f28fea96fafa114f2508ebef53735d7b6 |
| Headers | show |
| Series | some commit-graph leak fixes | expand |
On Tue, Oct 03, 2023 at 04:30:55PM -0400, Jeff King wrote: > diff --git a/commit-graph.c b/commit-graph.c > index 4aa2f294f1..744b7eb1a3 100644 > --- a/commit-graph.c > +++ b/commit-graph.c > @@ -2065,9 +2065,11 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) > free(graph_name); > } > > + free(ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); > ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(hash_to_hex(file_hash)); > final_graph_name = get_split_graph_filename(ctx->odb, > ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); > + free(ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1]); > ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1] = final_graph_name; > > result = rename(ctx->graph_name, final_graph_name); This hunk makes sense. It might be nice in the future to do something like: --- 8< --- diff --git a/commit-graph.c b/commit-graph.c index 5e8a3a5085..cadccbe276 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -59,7 +59,7 @@ void git_test_write_commit_graph_or_die(void) #define GRAPH_EXTRA_EDGES_NEEDED 0x80000000 #define GRAPH_EDGE_LAST_MASK 0x7fffffff -#define GRAPH_PARENT_NONE 0x70000000 +tdefine GRAPH_PARENT_NONE 0x70000000 #define GRAPH_LAST_EDGE 0x80000000 @@ -1033,11 +1033,11 @@ struct write_commit_graph_context { uint64_t progress_cnt; char *base_graph_name; - int num_commit_graphs_before; - int num_commit_graphs_after; - char **commit_graph_filenames_before; - char **commit_graph_filenames_after; - char **commit_graph_hash_after; + struct { + size_t nr; + char **fname; + char **hash; + } graphs_before, graphs_after; uint32_t new_num_commits_in_base; struct commit_graph *new_base_graph; --- >8 --- ...making the corresponding changes throughout the rest of the file. But that is definitely out of scope here, and could easily be left for another day. #leftoverbits Thanks, Taylor
On Thu, Oct 05, 2023 at 01:51:42PM -0400, Taylor Blau wrote: > @@ -1033,11 +1033,11 @@ struct write_commit_graph_context { > uint64_t progress_cnt; > > char *base_graph_name; > - int num_commit_graphs_before; > - int num_commit_graphs_after; > - char **commit_graph_filenames_before; > - char **commit_graph_filenames_after; > - char **commit_graph_hash_after; > + struct { > + size_t nr; > + char **fname; > + char **hash; > + } graphs_before, graphs_after; > uint32_t new_num_commits_in_base; > struct commit_graph *new_base_graph; > --- >8 --- > > ...making the corresponding changes throughout the rest of the file. But > that is definitely out of scope here, and could easily be left for > another day. I agree that it would make things a bit more readable, but there currently is no "hash_before". So they're not quite symmetric. -Peff
diff --git a/commit-graph.c b/commit-graph.c index 4aa2f294f1..744b7eb1a3 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2065,9 +2065,11 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) free(graph_name); } + free(ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1] = xstrdup(hash_to_hex(file_hash)); final_graph_name = get_split_graph_filename(ctx->odb, ctx->commit_graph_hash_after[ctx->num_commit_graphs_after - 1]); + free(ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1]); ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 1] = final_graph_name; result = rename(ctx->graph_name, final_graph_name);
When writing a split graph file, we replace the final element of the commit_graph_hash_after and commit_graph_filenames_after arrays. But since these are allocated strings, we need to free them before overwriting to avoid leaking the old string. Signed-off-by: Jeff King <peff@peff.net> --- commit-graph.c | 2 ++ 1 file changed, 2 insertions(+)