Re: [PATCH v8 2/5] am: stop exporting GIT_COMMITTER_DATE

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> For am and the sequencer fmt_ident() is in a function which is called
>> from a loop and it is convenient not to have to worry about passing an
>> strbuf around or allocating and freeing it on each function call
> 
> That's strbuf on caller's stack, right?  The actual string pointed
> by the strbuf's buf pointer will be needed and on the heap with or
> without the clean-up on top of the patch we are discussing, so I do
> not think alloc/free would be in the picture when considering the
> pros-and-cons.

>> The callers in ident (fmt_name(), git_author_info() and
>> git_committer_info()) return the string so they would need their own
>> strbuf or have to be changed so the caller passed one in. There are
>> quite a few callers of those functions and they seem to either
>> immediately call split_ident_line() or duplicate the returned string
>> (mostly the latter).
>>
>> So it would be a bit of work to do it but not an enormous amount
>> (assuming we don't change the signature of git_author_info() etc in
>> ident.c, although given the pattern of callers it might be worth doing
>> that if they are mostly duplicating the returned string anyway)
> 
> I'd say that is more than "while at it" clean-up.  It would be
> easier to review and slightly easier to do if done before this patch
> introduces rotating strbuf, but ...
> 
>> I'm going to be off line for 10-14 days from the beginning of next week,
>> I could take a look at it after that, or we could leave it for a future
>> improvement - what do you think?
> 
> ... I can be talked into punting it for later, at least for now.