git/log-tree.c, branch v2.45.4

Merge branch 'rs/date-mode-pass-by-value'

2024-04-16T21:50:29Z

The codepaths that reach date_mode_from_type() have been updated to pass "struct date_mode" by value to make them thread safe. * rs/date-mode-pass-by-value: date: make DATE_MODE thread-safe

date: make DATE_MODE thread-safe

2024-04-05T22:21:14Z

date_mode_from_type() modifies a static variable and returns a pointer to it. This is not thread-safe. Most callers of date_mode_from_type() use it via the macro DATE_MODE and pass its result on to functions like show_date(), which take a const pointer and don't modify the struct. Avoid the static storage by putting the variable on the stack and returning the whole struct date_mode. Change functions that take a constant pointer to expect the whole struct instead. Reduce the cost of passing struct date_mode around on 64-bit systems by reordering its members to close the hole between the 32-bit wide .type and the 64-bit aligned .strftime_fmt as well as the alignment hole at the end. sizeof reports 24 before and 16 with this change on x64. Keep .type at the top to still allow initialization without designator -- though that's only done in a single location, in builtin/blame.c. Signed-off-by: René Scharfe Acked-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'jk/pretty-subject-cleanup'

2024-04-01T20:21:34Z

Code clean-up in the "git log" machinery that implements custom log message formatting. * jk/pretty-subject-cleanup: format-patch: fix leak of empty header string format-patch: simplify after-subject MIME header handling format-patch: return an allocated string from log_write_email_headers() log: do not set up extra_headers for non-email formats pretty: drop print_email_subject flag pretty: split oneline and email subject printing shortlog: stop setting pp.print_email_subject

format-patch: fix leak of empty header string

2024-03-22T16:50:53Z

The log_write_email_headers() function recently learned to return the "extra_headers_p" variable to the caller as an allocated string. We start by copying rev_info.extra_headers into a strbuf, and then detach the strbuf at the end of the function. If there are no extra headers, we leave the strbuf empty. Likewise, if there are no headers to return, we pass back NULL. This misses a corner case which can cause a leak. The "do we have any headers to copy" check is done by looking for a NULL opt->extra_headers. But the "do we have a non-empty string to return" check is done by checking the length of the strbuf. That means if opt->extra_headers is the empty string, we'll "copy" it into the strbuf, triggering an allocation, but then leak the buffer when we return NULL from the function. We can solve this in one of two ways: 1. Rather than checking headers->len at the end, we could check headers->alloc to see if we allocated anything. That retains the original behavior before the recent change, where an empty extra_headers string is "passed through" to the caller. In practice this doesn't matter, though (the code which eventually looks at the result treats NULL or the empty string the same). 2. Only bother copying a non-empty string into the strbuf. This has the added bonus of avoiding a pointless allocation. Arguably strbuf_addstr() could do this optimization itself, though it may be slightly dangerous to do so (some existing callers may not get a fresh allocation when they expect to). In theory callers are all supposed to use strbuf_detach() in such a case, but there's no guarantee that this is the case. This patch uses option 2. Without it, building with SANITIZE=leak shows many errors in t4021 and elsewhere. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

format-patch: simplify after-subject MIME header handling

2024-03-20T00:54:16Z

In log_write_email_headers(), we append our MIME headers to the set of extra headers by creating a new strbuf, adding the existing headers, and then adding our new ones. We had to do it this way when our output buffer might point to the constant opt->extra_headers variable. But since the previous commit, we always make a local copy of that variable. Let's turn that into a strbuf, which lets the MIME code simply append to it. That simplifies the function and avoids a pointless extra copy of the headers. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

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

2024-03-20T00:54:16Z

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 Signed-off-by: Junio C Hamano

log: do not set up extra_headers for non-email formats

2024-03-20T00:54:16Z

The commit pretty-printer code has an "after_subject" parameter which it uses to insert extra headers into the email format. In show_log() we set this by calling log_write_email_headers() if we are using an email format, but otherwise default the variable to the rev_info.extra_headers variable. Since the pretty-printer code will ignore after_subject unless we are using an email format, this default is pointless. We can just set after_subject directly, eliminating an extra variable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

pretty: drop print_email_subject flag

2024-03-20T00:54:15Z

With one exception, the print_email_subject flag is set if and only if the commit format is email based: - in make_cover_letter() we set it along with CMIT_FMT_EMAIL explicitly - in show_log(), we set it if cmit_fmt_is_mail() is true. That covers format-patch as well as "git log --format=email" (or mboxrd). The one exception is "rev-list --format=email", which somewhat nonsensically prints the author and date as email headers, but no subject, like: $ git rev-list --format=email HEAD commit 64fc4c2cdd4db2645eaabb47aa4bac820b03cdba From: Jeff King Date: Tue, 19 Mar 2024 19:39:26 -0400 this is the subject this is the body It's doubtful that this is a useful format at all (the "commit" lines replace the "From" lines that would make it work as an actual mbox). But I think that printing the subject as a header (like this patch does) is the least surprising thing to do. So let's drop this field, making the code a little simpler and easier to reason about. Note that we do need to set the "rev" field of the pretty_print_context in rev-list, since that is used to check for subject_prefix, etc. It's not possible to set those fields via rev-list, so we'll always just print "Subject: ". But unless we pass in our rev_info, fmt_output_email_subject() would segfault trying to figure it out. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

commit-reach(repo_get_merge_bases): pass on "missing commits" errors

2024-02-29T16:06:01Z

The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases()` function (which is also surfaced via the `repo_get_merge_bases()` macro) is aware of that, too. Naturally, there are a lot of callers that need to be adjusted now, too. Next step: adjust the callers of `get_octopus_merge_bases()`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano

line-log.h: remove unnecessary include

2023-12-26T20:04:32Z

The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano