[08/19] line-log: always allocate the output prefix

Commit Message

Patrick Steinhardt May 29, 2024, 12:44 p.m. UTC

The returned string by `output_prefix()` is sometimes a string constant
and sometimes an allocated string. This has been fine until now because
we always leak the allocated strings, and thus we never tried to free
the string constant.

Fix the code to always return an allocated string and free the returned
value at all callsites.

Signed-off-by: Patrick Steinhardt <[e-mail]>
---
 line-log.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Junio C Hamano May 29, 2024, 7:51 p.m. UTC | #1

Patrick Steinhardt <[e-mail]> writes:

> The returned string by `output_prefix()` is sometimes a string constant
> and sometimes an allocated string. This has been fine until now because
> we always leak the allocated strings, and thus we never tried to free
> the string constant.
>
> Fix the code to always return an allocated string and free the returned
> value at all callsites.

Yay!

This leak always has bothered me, as it is not just once per process
invocation, but once per each commit that is shown.

Patch

diff --git a/line-log.c b/line-log.c
index d9bf2c8120..9a298209d0 100644
--- a/line-log.c
+++ b/line-log.c
@@ -899,14 +899,12 @@  static void print_line(const char *prefix, char first,
 
 static char *output_prefix(struct diff_options *opt)
 {
-	char *prefix = "";
-
 	if (opt->output_prefix) {
 		struct strbuf *sb = opt->output_prefix(opt, opt->output_prefix_data);
-		prefix = sb->buf;
+		return sb->buf;
+	} else {
+		return xstrdup("");
 	}
-
-	return prefix;
 }
 
 static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *range)
@@ -927,7 +925,7 @@  static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
 	const char *c_context = diff_get_color(opt->use_color, DIFF_CONTEXT);
 
 	if (!pair || !diff)
-		return;
+		goto out;
 
 	if (pair->one->oid_valid)
 		fill_line_ends(rev->diffopt.repo, pair->one, &p_lines, &p_ends);
@@ -1002,8 +1000,10 @@  static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
 				   c_context, c_reset, opt->file);
 	}
 
+out:
 	free(p_ends);
 	free(t_ends);
+	free(prefix);
 }
 
 /*
@@ -1012,7 +1012,11 @@  static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang
  */
 static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range)
 {
-	fprintf(rev->diffopt.file, "%s\n", output_prefix(&rev->diffopt));
+	char *prefix = output_prefix(&rev->diffopt);
+
+	fprintf(rev->diffopt.file, "%s\n", prefix);
+	free(prefix);
+
 	while (range) {
 		dump_diff_hacky_one(rev, range);
 		range = range->next;