Re: [PATCH] config: fix several access(NULL) calls

Junio C Hamano [off-list ref] writes:

Matthieu Moy [off-list ref] writes:

quoted

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 <redacted>
---

quoted

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/