Re: format-patch subject-prefix gets truncated when using the --numbered flag

Junio C Hamano [off-list ref] writes:

Adrian Dudau [off-list ref] writes:

quoted

I noticed that the --subject-prefix string gets truncated sometimes,
but only when using the --numbered flat. Here's an example:

addu@sestofb11:/data/fb/addu/git$ export longm="very very very very
very very very very very very very very very very long prefix"

This looks like "dr, my arm hurts when I twist it this way---don't
do that then" ;-).  Truncation is designed and intended to preserve
space for the real title of commit.

Having said that...
...
I think this is just an old oversight.  The latter should have been
even shorter than the former one (or the former should be longer
than the latter) to account for the width of the counter e.g. 01/27.

And having said all that, if we really want to allow overlong
subject prefix that would end up hiding the real title of the patch,
a modern way to do so would be to use xstrfmt() like the attached
toy-patch does.  Note that this is merely a small first step---you'd
notice that "subject" is kept around as a "static" and only freed
upon entry to this function for the second time, to preserve the
ownership model of the original code.  In a real "fix" (if this
needs to be "fixed", that is), I think the ownership model of the
storage used for *subject_p and *extra_headers_p needs to be updated
so that it will become caller's responsibility to free them
(similarly, the ownership model of opt->diffopt.stat_sep that is
assigned the address of the static buffer[] in the same function
needs to be revisited).

That "buffer" thing I think would need to be a bit more careful even
in the current code, which _does_ use snprintf() correctly to avoid
overflowing the buffer[], by the way.  If you have an overlong
opt->mime_boundary, the resulting "e-mail" looking output can become
structurely broken.  The truncation may happen way before the full
line for Content-Transfer-Encoding: is written, for example.

So this function seems to have a lot more graver problems that need
to be looked at.

 log-tree.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 8c2415747a..24c98f5a80 100644
--- a/log-tree.c
+++ b/log-tree.c

@@ -337,29 +337,23 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			     const char **extra_headers_p,
 			     int *need_8bit_cte_p)
 {
-	const char *subject = NULL;
+	static const char *subject = NULL;
 	const char *extra_headers = opt->extra_headers;
 	const char *name = oid_to_hex(opt->zero_commit ?
 				      &null_oid : &commit->object.oid);
 
+	free((void *)subject);
 	*need_8bit_cte_p = 0; /* unknown */
 	if (opt->total > 0) {
-		static char buffer[64];
-		snprintf(buffer, sizeof(buffer),
-			 "Subject: [%s%s%0*d/%d] ",
-			 opt->subject_prefix,
-			 *opt->subject_prefix ? " " : "",
-			 digits_in_number(opt->total),
-			 opt->nr, opt->total);
-		subject = buffer;
+		subject = xstrfmt("Subject: [%s%s%0*d/%d] ",
+				  opt->subject_prefix,
+				  *opt->subject_prefix ? " " : "",
+				  digits_in_number(opt->total),
+				  opt->nr, opt->total);
 	} else if (opt->total == 0 && opt->subject_prefix && *opt->subject_prefix) {
-		static char buffer[256];
-		snprintf(buffer, sizeof(buffer),
-			 "Subject: [%s] ",
-			 opt->subject_prefix);
-		subject = buffer;
+		subject = xstrfmt("Subject: [%s] ", opt->subject_prefix);
 	} else {
-		subject = "Subject: ";
+		subject = xstrdup("Subject: ");
 	}
 
 	fprintf(opt->diffopt.file, "From %s Mon Sep 17 00:00:00 2001\n", name);