diff: store graph prefix buf in git_graph struct

The diffopt output_prefix interface makes it the callback's job to handle ownership of the memory it returns, keeping it valid while callers are using it and then eventually freeing it when we are done diffing. In diff_output_prefix_callback() we handle this with a static strbuf, effectively "leaking" it when the diff is done (but not triggering any leak detectors because it's technically still reachable). This has not been a big problem in practice, but it is a problem for libification: two diffs running in the same process could stomp on each other's prefix buffers. Since we only need the strbuf when we are formatting graph padding, we can give ownership of the strbuf to the git_graph struct, letting us free it when that struct is no longer in use. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 3, 2024 at 17:13 UTC 1164e270b5af80516625b628945ec7365d992055
1 file changed +12 -6
graph.c
+12 -6
@@ -307,24 +307,28 @@ struct git_graph {
307 * stored as an index into the array column_colors.
308 */
309 unsigned short default_column_color;
310 +
311 + /*
312 + * Scratch buffer for generating prefixes to be used with
313 + * diff_output_prefix_callback().
314 + */
315 + struct strbuf prefix_buf;
316 };
317
318 static const char *diff_output_prefix_callback(struct diff_options *opt, void *data)
319 {
320 struct git_graph *graph = data;
315 - static struct strbuf msgbuf = STRBUF_INIT;
321
322 assert(opt);
323
324 if (!graph)
325 return opt->line_prefix;
326
322 - strbuf_reset(&msgbuf);
327 + strbuf_reset(&graph->prefix_buf);
328 if (opt->line_prefix)
324 - strbuf_addstr(&msgbuf, opt->line_prefix);
325 - if (graph)
326 - graph_padding_line(graph, &msgbuf);
327 - return msgbuf.buf;
329 + strbuf_addstr(&graph->prefix_buf, opt->line_prefix);
330 + graph_padding_line(graph, &graph->prefix_buf);
331 + return graph->prefix_buf.buf;
332 }
333
334 static const struct diff_options *default_diffopt;
@@ -394,6 +398,7 @@ struct git_graph *graph_init(struct rev_info *opt)
398 * The diff output prefix callback, with this we can make
399 * all the diff output to align with the graph lines.
400 */
401 + strbuf_init(&graph->prefix_buf, 0);
402 opt->diffopt.output_prefix = diff_output_prefix_callback;
403 opt->diffopt.output_prefix_data = graph;
404
@@ -409,6 +414,7 @@ void graph_clear(struct git_graph *graph)
414 free(graph->new_columns);
415 free(graph->mapping);
416 free(graph->old_mapping);
417 + strbuf_release(&graph->prefix_buf);
418 free(graph);
419 }
420