Re: [PATCH v2] Add a Windows-specific fallback to getenv("HOME");

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

 



Am 05.06.2014 10:03, schrieb Stepan Kasal:
> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> Date: Wed, 2 Jun 2010 00:41:33 +0200
> 
> If HOME is not set, use $HOMEDRIVE$HOMEPATH
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> Signed-off-by: Stepan Kasal <kasal@xxxxxx>
> ---
> 
> Hello Karsten,
> thanks for your explanation.  There are more things to be done, but
> I hope you can ack this patch as a step forward.
> 

No, not really. Its sure better than introducing a special get_home_directory(), but it still increases the diff between upstream and msysgit rather than reducing it. The main critique points still remain:

 * $HOME is usually set up correctly before calling git, so this is essentially dead code (just checked, portable git's git-bash.bat and git-cmd.bat also do this correctly)

 * even if $HOME was empty, git should setenv("HOME") so that child processes can benefit from it (similar to TMPDIR and TERM in current msysgit's mingw_startup()). Not setting $HOME because it may hypothetically break child processes is a very weak argument, as we always did set $HOME in etc/profile (since the initial version back in 2007).

 * no fallback to $USERPROFILE doesn't work with diconnected home share

If you really have time to spare, I suggest you focus on getting the Unicode patches upstream so that we can progress from there (e.g. move $HOME setup to mingw_startup() so that we can get rid of redundant logic in etc/profile, git-wrapper, git-bash.bat, git-cmd.bat etc.).

> Hello Dscho,
> I hope you can ack this as well: it is basically equivalent with your
> patch, tailored according to current upstream fashion,  ;-)
> 
> Stepan
> 
>  compat/mingw.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index a0e13bc..e108388 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1181,6 +1181,11 @@ char *mingw_getenv(const char *name)
>  		if (!result)
>  			result = getenv_cs("TEMP");
>  	}

else?

> +	if (!result && !strcmp(name, "HOME")) {
> +		struct strbuf buf = STRBUF_INIT;
> +		strbuf_addf(&buf, "%s%s", getenv_cs("HOMEDRIVE"), getenv_cs("HOMEPATH"));

No surplus '/', good!

> +		result = strbuf_detach(&buf, NULL);

This leaks memory.

> +	}
>  	return result;
>  }
>  
> 

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