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

[Junio C Hamano <gitster@pobox.com>]

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.

Thanks.