Junio C Hamano <gits...@pobox.com> writes:

> Matthieu Moy <matthieu....@imag.fr> 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....@imag.fr>
>> ---
>>> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to