Junio C Hamano wrote:
> Huynh Khoi Nguyen NGUYEN <Huynh-Khoi-Nguyen.Nguyen@xxxxxxxxxxxxxxx>
[...]
>> 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).
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html