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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: <hidden>
Date: 2016-06-15 22:53:58

Ramsay Jones [off-list ref] 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()",  
quoted
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).

quoted
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) ?

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

From: Ramsay Jones <hidden>
Date: 2016-06-15 22:53:59

nguyenhu@minatec.inpg.fr wrote:
Ramsay Jones [off-list ref] writes:
quoted
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()",  
quoted
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).

quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help