From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 30 Jun 2022 16:36:34 GMT
Subject: Re: [PATCH 3/3] cat-file: add mailmap support
Message-ID: <9b33bbc3-1781-6655-d6ef-f0793bdd0951@gmail.com>
In-Reply-To: <61074b4c-c48f-da89-5d03-b280b9c4fedf@gmail.com>

On 30/06/2022 16:50, Phillip Wood wrote:
>> +char *replace_idents_using_mailmap(char *object_buf, size_t *size)
>> +{
>> +    struct strbuf sb = STRBUF_INIT;
>> +    strbuf_attach(&sb, object_buf, *size, *size + 1);
> 
> I'm worried by this as I don't think we really own the buffer returned 
> by read_object_file(). git maintains a cache of objects it has loaded 
> and if this strbuf grows when the author is rewritten then the pointer 
> stored in the cache will become invalid. If you look at the code in 
> revision.c you'll see that commit_rewrite_person() is called on a copy 
> of the original object.

Sorry ignore that, looking through the code again we do own object_buf 
so this is fine.

Sorry for the confusion

Phillip

>> +    rewrite_ident_line(&sb, "\nauthor ", &mailmap);
>> +    rewrite_ident_line(&sb, "\ncommitter ", &mailmap);
>> +    rewrite_ident_line(&sb, "\ntagger ", &mailmap);
>> +    *size = sb.len;
>> +    return strbuf_detach(&sb, NULL);
>> +}
>> [...]
>> +test_expect_success '--no-use-mailmap disables mailmap in cat-file' '
>> +    test_when_finished "rm .mailmap" &&
>> +    cat >.mailmap <<-EOF &&
>> +    A U Thor <author@example.com> Orig <orig@example.com>
>> +    EOF
>> +    cat >expect <<-EOF &&
>> +    author Orig <orig@example.com>
>> +    EOF
>> +    git cat-file --no-use-mailmap commit HEAD >log &&
>> +    grep author log >actual &&
>> +    sed -e "/^author/q" actual >log &&
> 
> This line does not have any effect on the contents of log
> 
>> +    sed -e "s/ [0-9][0-9]* [-+][0-9][0-9][0-9][0-9]$//" log >actual &&
> 
> I think you can simplify this series of commands to do
>      git cat-file ... >log
>      sed -n "/^author /s/\([^>]*>\).*/\1/p" log >actual
> 
> Best Wishes
> 
> Phillip