Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config

nguyenhu@minatec.inpg.fr· Jun 1, 2012, 19:49 UTC · lore

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 23 quoted lines

> I have not tried this patch (or the v3 version, which I haven't read  
> yet), but
> is it likely that this has re-introduced the bug addressed by commit 05bab3ea
> ("config.c: Fix a static buffer overwrite bug by avoiding mkpath()",  
> >19-11-2011)?.
>
> I don't know the answer, but I suspect that it may have done just  
> that. >(indeed, it
> may well have made the bug more likely to appear).
>
>
>> The original that read from $HOME/.gitconfig was simple enough so
>> having three copies of getenv("HOME") was perfectly fine, but as you
>> are introduce this much complexity to to decide which two files to
>> read from, the code added this patch needs to be refactored and
>> three copies of the same logic need to be consolidated, I would have
>> to say.
>
> I agree. Also, using mksnpath() in the refactored code (rather than
> mkpath()) would be a good idea. :-P
>
> ATB,
> Ramsay Jones

Is not mkpath() the same function as mksnpath with char *buff = buf[PATH_MAX] and size_t n = sizeof(buf) ?

Ramsay Jones· Jun 4, 2012, 17:27 UTC · re: nguyenhu@minatec.inpg.fr · lore

nguyenhu@minatec.inpg.fr wrote:

Show 25 quoted lines

> Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
>> I have not tried this patch (or the v3 version, which I haven't read  
>> yet), but
>> is it likely that this has re-introduced the bug addressed by commit 05bab3ea
>> ("config.c: Fix a static buffer overwrite bug by avoiding mkpath()",  
>>> 19-11-2011)?.
>> I don't know the answer, but I suspect that it may have done just  
>> that. >(indeed, it
>> may well have made the bug more likely to appear).
>>
>>
>>> The original that read from $HOME/.gitconfig was simple enough so
>>> having three copies of getenv("HOME") was perfectly fine, but as you
>>> are introduce this much complexity to to decide which two files to
>>> read from, the code added this patch needs to be refactored and
>>> three copies of the same logic need to be consolidated, I would have
>>> to say.
>> I agree. Also, using mksnpath() in the refactored code (rather than
>> mkpath()) would be a good idea. :-P
>>
>> ATB,
>> Ramsay Jones
> 
> Is not mkpath() the same function as mksnpath with char *buff =  
> buf[PATH_MAX] and size_t n = sizeof(buf) ?

I'm sorry but I just can't understand your question. :(

Have you looked at commit 05bab3ea? Is the commit message unclear?

The main difference between mkpath() and mksnpath(), as far as this bug is concerned, is that mkpath() returns a reference to *recycled* internal static buffers, whereas mksnpath() does not (you provide your own).

This evening I finally had a look at your patch, well v4 of the patch, and I can confirm that it does indeed re-introduce the bug. I will reply to the v4 patch email with more comments.

HTH

ATB, Ramsay Jones