format-patch: return an allocated string from log_write_email_headers()

When pretty-printing a commit in the email format, we have to fill in the "after subject" field of the pretty_print_context with any extra headers the user provided (e.g., from "--to" or "--cc" options) plus any special MIME headers. We return an out-pointer that sometimes points to a newly heap-allocated string and sometimes not. To avoid leaking, we store the allocated version in a buffer with static lifetime, which is ugly. Worse, as we extend the header feature, we'll end up having to repeat this ugly pattern. Instead, let's have our out-pointer pass ownership back to the caller, and duplicate the string when necessary. This does mean one extra allocation per commit when you use extra headers, but in the context of format-patch which is showing diffs, I don't think that's even measurable. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 19, 2024 at 20:35 UTC 305a68143cc0e9b714d71417efa9f0162dd07221
4 files changed +9 -7
builtin/log.c
+1
@@ -1370,6 +1370,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
1370 encoding, need_8bit_cte);
1371 fprintf(rev->diffopt.file, "%s\n", sb.buf);
1372
1373 + free(pp.after_subject);
1374 strbuf_release(&sb);
1375
1376 shortlog_init(&log);
log-tree.c
+6 -5
@@ -470,11 +470,11 @@ void fmt_output_email_subject(struct strbuf *sb, struct rev_info *opt)
470 }
471
472 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
473 - const char **extra_headers_p,
473 + char **extra_headers_p,
474 int *need_8bit_cte_p,
475 int maybe_multipart)
476 {
477 - const char *extra_headers = opt->extra_headers;
477 + char *extra_headers = xstrdup_or_null(opt->extra_headers);
478 const char *name = oid_to_hex(opt->zero_commit ?
479 null_oid() : &commit->object.oid);
480
@@ -496,12 +496,11 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
496 graph_show_oneline(opt->graph);
497 }
498 if (opt->mime_boundary && maybe_multipart) {
499 - static struct strbuf subject_buffer = STRBUF_INIT;
499 + struct strbuf subject_buffer = STRBUF_INIT;
500 static struct strbuf buffer = STRBUF_INIT;
501 struct strbuf filename = STRBUF_INIT;
502 *need_8bit_cte_p = -1; /* NEVER */
503
504 - strbuf_reset(&subject_buffer);
504 strbuf_reset(&buffer);
505
506 strbuf_addf(&subject_buffer,
@@ -519,7 +518,8 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
518 extra_headers ? extra_headers : "",
519 mime_boundary_leader, opt->mime_boundary,
520 mime_boundary_leader, opt->mime_boundary);
522 - extra_headers = subject_buffer.buf;
521 + free(extra_headers);
522 + extra_headers = strbuf_detach(&subject_buffer, NULL);
523
524 if (opt->numbered_files)
525 strbuf_addf(&filename, "%d", opt->nr);
@@ -854,6 +854,7 @@ void show_log(struct rev_info *opt)
854
855 strbuf_release(&msgbuf);
856 free(ctx.notes_message);
857 + free(ctx.after_subject);
858
859 if (cmit_fmt_is_mail(ctx.fmt) && opt->idiff_oid1) {
860 struct diff_queue_struct dq;
log-tree.h
+1 -1
@@ -29,7 +29,7 @@ void format_decorations(struct strbuf *sb, const struct commit *commit,
29 int use_color, const struct decoration_options *opts);
30 void show_decorations(struct rev_info *opt, struct commit *commit);
31 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
32 - const char **extra_headers_p,
32 + char **extra_headers_p,
33 int *need_8bit_cte_p,
34 int maybe_multipart);
35 void load_ref_decorations(struct decoration_filter *filter, int flags);
pretty.h
+1 -1
@@ -35,7 +35,7 @@ struct pretty_print_context {
35 */
36 enum cmit_fmt fmt;
37 int abbrev;
38 - const char *after_subject;
38 + char *after_subject;
39 int preserve_subject;
40 struct date_mode date_mode;
41 unsigned date_mode_explicit:1;