Junio C Hamano <gits...@pobox.com> writes:

> Adrian Dudau <adrian.du...@enea.com> 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);

Reply via email to