Re: [PATCH/WIP v3 07/31] am: extract patch, message and authorship with git-mailinfo

From: Paul Tan <pyokagan@gmail.com>
Date: Jun 19, 2015, 09:22 UTC
Message-ID: <CACRoPnS9eyBF5NEM7sKvep+A8aKUNLJDaV-1c_oWDBwMcv26Bg@mail.gmail.com>
In-Reply-To: <xmqqtwu4d8pg.fsf@gitster.dls.corp.google.com>

On Fri, Jun 19, 2015 at 5:10 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Paul Tan <pyokagan@gmail.com> writes:
>
>> +     /* commit message and metadata */
>> +     struct strbuf author_name;
>> +     struct strbuf author_email;
>> +     struct strbuf author_date;
>> +     struct strbuf msg;
>
> Same comment as "dir" in the earlier patch applies to these.  If the
> fields are read or computed and then kept constant, use a temporary
> variable that is a strbuf to read/compute the final value, and then
> detach to a "const char *" field.  If they are constantly changing
> and in-place updates are vital, then they can and should be strbufs,
> but I do not think that is the case for these.

I do think it is the case here. The commit message and metadata fields change for every patch we process, and we could be processing a large volume of patches, so we must be careful to not leak any memory.

We could use a char* field, but then to prevent memory leaks we would then have to free() it and malloc() a new string for every patch we process, which will lead to lots of malloc()s and free()s over a large volume of patches that can lead to memory fragmentation.

Regards, Paul