Re: [PATCH] cat-file: fix mailmap application for different author and committer

From: Christian Couder <christian.couder@gmail.com>
Date: Jun 11, 2025, 09:38 UTC
Message-ID: <CAP8UFD1hBo_ZH1nyGBOoQOwx=44CfnkhEOHYu3_XMwSzXQAHdg@mail.gmail.com>
In-Reply-To: <20250611062643.8639-1-siddharthasthana31@gmail.com>

On Wed, Jun 11, 2025 at 8:27 AM <siddharthasthana31@gmail.com> wrote:

> Reported-by: Vasilii Iakliushin <viakliushin@gitlab.com>
> Reviewed-by: Christian Couder <christian.couder@gmail.com>

Nit: I reviewed it when you suggested it on a GitLab MR (Merge
Request), but I am not sure it counts unless I also review it here. I
think the "Reviewed-by: ..." trailer is for patches reviewed on the
regular Git mailing list (or maybe on the private Git security list).
So maybe "Helped-by: ..." would have been better in this case.

Anyway I have now reviewed it again and I found it great.

Thanks for working on this!

Show 18 quoted lines

> Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
> ---
>  ident.c            |  4 ++++
>  t/t4203-mailmap.sh | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
>
> diff --git a/ident.c b/ident.c
> index 967895d885..281e830573 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -412,6 +412,10 @@ void apply_mailmap_to_header(struct strbuf *buf, const char **header,
>                                 found_header = 1;
>                                 buf_offset += endp - line;
>                                 buf_offset += rewrite_ident_line(person, endp - person, buf, mailmap);
> +                               /* Recompute endp after potential buffer reallocation */
> +                               endp = buf->buf + buf_offset;
> +                               if (*endp == '\n')
> +                                       buf_offset++;

Yeah, without this, in the next iteration of the `for (;;) { ... }` loop after the "author" header has been found, we have:

        line = buf->buf + buf_offset;

which sets `line` to something like "\ncommitter C O Mitter <committer@example.com> ...", and then:

        if (!*line || *line == '\n')
            return; /* End of headers */

which just returns as `*line` is indeed '\n'.

>                                 break;
>                         }