Junio C Hamano <gitster@xxxxxxxxx> writes:
> Adrian Dudau <Adrian.Dudau@xxxxxxxx> writes:
>
>> 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);