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

[Johannes Schindelin <johannes.schindelin@gmx.de>]

Hi Paul,

On 2015-06-19 18:15, Paul Tan wrote:
> On Fri, Jun 19, 2015 at 11:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 
>> The primary thing I care about is to discourage callers of the API element
>> am_state from touching these fields with strbuf functions. If it is char * then
>> the would think twice before modifying (which would involve allocating the
>> new buffer, forming the new value and assigning into it). With the fields left
>> as strbuf after they are read or formed by the API (by reading by the API
>> implementation from $GIT_DIR/rebase-apply/), it is too tempting to do
>> strbuf_add(), strbuf_trim(), etc., without restoring the value to the original
>> saying "I'm the last user of it anyway"--that is the sloppiness we can prevent
>> by not leaving it as strbuf.
> 
> I don't think this is a good deterrent. If the code wanted to, it
> could just use strbuf_attach()/strbuf_detach() as well, no?

Sadly, I am a bit too busy with some loose Git for Windows ends, so I haven't had the chance to look at your code.

But I still wanted to throw this idea out there: would it maybe possible to avoid having those values as global variables, and instead pass them as const char * parameters to the respective functions? That should help resolve the concerns of both sides because it would allow us to keep the strbuf instances, just as local variables.

Ciao,
Dscho