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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Paul,

On 2015-06-19 18:15, Paul Tan wrote:
> On Fri, Jun 19, 2015 at 11:09 PM, Junio C Hamano <gitster@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]