Re: [PATCH 1/4 v8] config: read (but not write) from $XDG_CONFIG_HOME/git/config file

From: Thomas Rast <trast@student.ethz.ch>
Date: Jul 12, 2012, 07:55 UTC
Message-ID: <877gu9wh05.fsf@thomas.inf.ethz.ch>
In-Reply-To: <1340355806-6894-2-git-send-email-Matthieu.Moy@imag.fr>

Hi all,

This patch causes valgrind warnings in t1300.81 (get --path copes with unset $HOME) about passing NULL to access():

==25286== Syscall param access(pathname) points to unaddressable byte(s) ==25286== at 0x56E2227: access (in /lib64/libc-2.14.1.so) ==25286== by 0x48CA42: git_config_early (config.c:948) ==25286== by 0x4F0C20: check_repository_format_gently (setup.c:369) ==25286== by 0x4F1A52: setup_git_directory_gently_1 (setup.c:531) ==25286== by 0x4F24ED: setup_git_directory_gently (setup.c:725) ==25286== by 0x405D8C: run_builtin (git.c:287) ==25286== by 0x40548E: main (git.c:467) ==25286== Address 0x0 is not stack'd, malloc'd or (recently) free'd

AFAICT it is this bit of code:

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 9 quoted lines

> +void home_config_paths(char **global, char **xdg, char *file)
> +{
> +	char *xdg_home = getenv("XDG_CONFIG_HOME");
> +	char *home = getenv("HOME");
> +	char *to_free = NULL;
> +
> +	if (!home) {
> +		if (global)
> +			*global = NULL;

[...]

Show 32 quoted lines

>  int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  {
>  	int ret = 0, found = 0;
> -	const char *home = NULL;
> +	char *xdg_config = NULL;
> +	char *user_config = NULL;
> +
> +	home_config_paths(&user_config, &xdg_config, "config");
>  
>  	if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
>  		ret += git_config_from_file(fn, git_etc_gitconfig(),
> @@ -937,14 +940,14 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>  		found += 1;
>  	}
>  
> -	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);
> -			found += 1;
> -		}
> +	if (!access(xdg_config, R_OK)) {
> +		ret += git_config_from_file(fn, xdg_config, data);
> +		found += 1;
> +	}
> +
> +	if (!access(user_config, R_OK)) {
> +		ret += git_config_from_file(fn, user_config, data);
> +		found += 1;
>  	}

That is, while the old code was careful about home==NULL, the new code checks this only in home_config_paths(); after that, it does no further NULL check.

There's a similar instance in your changes to cmd_config(), found by running 'unset HOME; valgrind git config --global --get foo.bar':

==27841== Syscall param access(pathname) points to unaddressable byte(s) ==27841== at 0x56E2227: access (in /lib64/libc-2.14.1.so) ==27841== by 0x425280: cmd_config (config.c:390) ==27841== by 0x405C34: run_builtin (git.c:306) ==27841== by 0x40548E: main (git.c:467) ==27841== Address 0x0 is not stack'd, malloc'd or (recently) free'd

around here:

> @@ -379,13 +382,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)

[...]

Show 6 quoted lines

> +		char *user_config = NULL;
> +		char *xdg_config = NULL;
> +
> +		home_config_paths(&user_config, &xdg_config, "config");
> +
> +		if (access(user_config, R_OK) && !access(xdg_config, R_OK) &&

Can you fix this?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch