Re: [PATCHv2] Possibility to read both from ~/.gitconfig and from $XDG_CONFIG_HOME/git/config
- From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
- Date
- May 31, 2012, 22:06 UTC
- Message-ID
- <4FC7EB78.5090903@ramsay1.demon.co.uk>
- In-Reply-To
- <7vr4u1xrkp.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
[...]
Show 23 quoted lines
>> diff --git a/config.c b/config.c
>> index 71ef171..53557dc 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -939,10 +939,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>>
>> home = getenv("HOME");
>> if (home) {
>> - char buf[PATH_MAX];
>> - char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
>> - if (!access(user_config, R_OK)) {
>> - ret += git_config_from_file(fn, user_config, data);
>> + const char *gitconfig_path = xstrdup(mkpath("%s/.gitconfig", home));
>> + const char *xdg_config_path = NULL;
>> + const char *xdg_config_home = NULL;
>> +
>> + xdg_config_home = getenv("XDG_CONFIG_HOME");
>> + if (xdg_config_home)
>> + xdg_config_path = xstrdup(mkpath("%s/git/config", xdg_config_home));
>> + else
>> + xdg_config_path = xstrdup(mkpath("%s/.config/git/config", home));
>
> Exactly the same comment applies here, too.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).
Show 7 quoted lines
>
> 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