Junio C Hamano <gitster@xxxxxxxxx> writes:
> Matthieu Moy <Matthieu.Moy@xxxxxxx> writes:
>
>> When $HOME is unset, home_config_paths fails and returns NULL pointers
>> for user_config and xdg_config. Valgrind complains with Syscall param
>> access(pathname) points to unaddressable byte(s).
>>
>> Don't call blindly access() on these variables, but test them for
>> NULL-ness before.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@xxxxxxx>
>> ---
>>> This patch causes valgrind warnings in t1300.81 (get --path copes with
>>> unset $HOME) about passing NULL to access():
>>
>> Indeed. The following patch should fix it.
>>
>> builtin/config.c | 3 ++-
>> config.c | 4 ++--
>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/builtin/config.c b/builtin/config.c
>> index e8e1c0a..67945b2 100644
>> --- a/builtin/config.c
>> +++ b/builtin/config.c
>> @@ -387,7 +387,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>>
>> home_config_paths(&user_config, &xdg_config, "config");
>>
>> - if (access(user_config, R_OK) && !access(xdg_config, R_OK))
>> + if (user_config && access(user_config, R_OK) &&
>> + xdg_config && !access(xdg_config, R_OK))
>> given_config_file = xdg_config;
>
> Shouldn't we be using xdg_config, if user_config is NULL and
> xdg_config is defined and accessible?
I don't think so. If user_config is NULL, it means something went wrong,
because $HOME is unset. In this case, I'd rather die than using some
other configuration file silently (which would be possible if
$XDG_CONFIG_HOME is set), and this is what the code does:
if (user_config && access(user_config, R_OK) &&
xdg_config && !access(xdg_config, R_OK))
given_config_file = xdg_config;
else if (user_config)
given_config_file = user_config;
else
die("$HOME not set");
Perhaps it could actually be made even clearer with
if (!user_config)
die("$HOME not set");
else if (access(user_config, R_OK) &&
xdg_config && !access(xdg_config, R_OK))
given_config_file = xdg_config;
else
given_config_file = user_config;
That said, I don't care very strongly about it.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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