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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:

> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> 	if (use_global_config) {
>>                 if (is $HOME/.gitconfig usable?) {
>> 			use it;
>
> Yes, but when $HOME is unset, the question doesn't really make sense.
> Maybe the file exists, but we can't know since the user broke his
> configuration by unsetting $HOME. The intent was to avoid writing to the
> XDG file unless it was very clear that the user wanted it, so in doubt,
> dying seems the best option.

I would think that it is plausible that the user wanted to write
into XDG one and used "unset HOME" as a way to signal that wish. 

Are there ways to force writing into XDG ones without having to
remove the $HOME/ ones (perhaps the user wants to keep them for use
with older versions of Git on a different machine that shares the
same $HOME directory)?  Temporarily unsetting HOME may be how a user
might achieve it.

If you want to disallow such a use case, that is fine, but at least
the logic needs to be described in comment.  Perhaps based on one of
your rewrites earlier in the thread, like this?

 builtin/config.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index 67945b2..a788409 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -387,13 +387,26 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
 		home_config_paths(&user_config, &xdg_config, "config");
 
-		if (user_config && access(user_config, R_OK) &&
+		if (!user_config)
+			/*
+			 * We do not know HOME/.gitconfig exists or
+			 * not, hence we do not know if we should
+			 * write to XDG location, so we error out even
+			 * if XDG_CONFIG_HOME is set and points at a
+			 * sane location.
+			 *
+			 * In other words, we forbid the user from
+			 * telling us to write to XDG location,
+			 * pretending that $HOME/.gitconfig does not
+			 * exist, by temporarily unsetting HOME.
+			 */
+			die("$HOME not set");
+
+		if (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");
+			given_config_file = user_config;
 	}
 	else if (use_system_config)
 		given_config_file = git_etc_gitconfig();
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]