* [PATCH 2/3] mingw.c: move definition of mingw_getenv down
2011-06-06 6:57 [PATCH 0/3] sh-i18n--envsubst; case-sensitive getenv on Windows Johannes Sixt
2011-06-06 6:59 ` [PATCH 1/3] sh-i18n--envsubst: do not crash when no arguments are given Johannes Sixt
@ 2011-06-06 7:06 ` Johannes Sixt
2011-06-06 7:08 ` [PATCH 3/3] Windows: teach getenv to do a case-sensitive search Johannes Sixt
2 siblings, 0 replies; 5+ messages in thread
From: Johannes Sixt @ 2011-06-06 7:06 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, Git Mailing List, Erik Faye-Lund,
Johannes Schindelin
From: Johannes Sixt <j6t@kdbg.org>
We want to use static lookup_env() in a subsequent change.
At first sight, this change looks innocent. But it is not due to the
#undef getenv. There is one caller of getenv between the old location and
the new location whose behavior could change. But as can be seen from the
defintion of mingw_getenv, the behavior for this caller does not change
substantially.
To ensure consistent behavior in the future, change all getenv callers
in mingw.c to use mingw_getenv.
With this patch, this is not a big deal, yet, but with the subsequent
change, where we teach getenv to do a case-sensitive lookup, the behavior
of all call sites is changed.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Erik, Dscho,
the 4msysgit tree has additional calls of getenv in mingw.c. You might
want to update them to call mingw_getenv() instead.
But then not doing so would not have a dramatic effect. It is similar
to GIT_ASK_YESNO: People can still set Git_Ask_YesNo, and it will
work in the same way as usual, *unless* they also have GIT_ASK_YESNO
set as well (and with a different value).
compat/mingw.c | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index e085e8b..ee480f9 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -178,7 +178,7 @@ static int ask_yes_no_if_possible(const char *format, ...)
vsnprintf(question, sizeof(question), format, args);
va_end(args);
- if ((retry_hook[0] = getenv("GIT_ASK_YESNO"))) {
+ if ((retry_hook[0] = mingw_getenv("GIT_ASK_YESNO"))) {
retry_hook[1] = question;
return !run_command_v_opt(retry_hook, 0);
}
@@ -599,19 +599,6 @@ char *mingw_getcwd(char *pointer, int len)
return ret;
}
-#undef getenv
-char *mingw_getenv(const char *name)
-{
- char *result = getenv(name);
- if (!result && !strcmp(name, "TMPDIR")) {
- /* on Windows it is TMP and TEMP */
- result = getenv("TMP");
- if (!result)
- result = getenv("TEMP");
- }
- return result;
-}
-
/*
* See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
* (Parsing C++ Command-Line Arguments)
@@ -711,7 +698,7 @@ static const char *parse_interpreter(const char *cmd)
*/
static char **get_path_split(void)
{
- char *p, **path, *envpath = getenv("PATH");
+ char *p, **path, *envpath = mingw_getenv("PATH");
int i, n = 0;
if (!envpath || !*envpath)
@@ -1128,6 +1115,19 @@ char **make_augmented_environ(const char *const *vars)
return env;
}
+#undef getenv
+char *mingw_getenv(const char *name)
+{
+ char *result = getenv(name);
+ if (!result && !strcmp(name, "TMPDIR")) {
+ /* on Windows it is TMP and TEMP */
+ result = getenv("TMP");
+ if (!result)
+ result = getenv("TEMP");
+ }
+ return result;
+}
+
/*
* Note, this isn't a complete replacement for getaddrinfo. It assumes
* that service contains a numerical port, or that it is null. It
--
1.7.6.rc0.1186.gfb4fd
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 3/3] Windows: teach getenv to do a case-sensitive search
2011-06-06 6:57 [PATCH 0/3] sh-i18n--envsubst; case-sensitive getenv on Windows Johannes Sixt
2011-06-06 6:59 ` [PATCH 1/3] sh-i18n--envsubst: do not crash when no arguments are given Johannes Sixt
2011-06-06 7:06 ` [PATCH 2/3] mingw.c: move definition of mingw_getenv down Johannes Sixt
@ 2011-06-06 7:08 ` Johannes Sixt
2011-10-05 16:21 ` Karsten Blees
2 siblings, 1 reply; 5+ messages in thread
From: Johannes Sixt @ 2011-06-06 7:08 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, Git Mailing List, Erik Faye-Lund,
Johannes Schindelin
From: Johannes Sixt <j6t@kdbg.org>
getenv() on Windows looks up environment variables in a case-insensitive
manner. Even though all documentations claim that the environment is
case-insensitive, it is possible for applications to pass an environment
to child processes that has variables that differ only in case. Bash on
Windows does this, for example, and sh-i18n--envsubst depends on this
behavior.
With this patch environment variables are first looked up in a
case-sensitive manner; only if this finds nothing, the system's getenv() is
used as a fallback.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
compat/mingw.c | 23 ++++++++++++++++++++---
1 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index ee480f9..6e5af32 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1116,14 +1116,31 @@ char **make_augmented_environ(const char *const *vars)
}
#undef getenv
+
+/*
+ * The system's getenv looks up the name in a case-insensitive manner.
+ * This version tries a case-sensitive lookup and falls back to
+ * case-insensitive if nothing was found. This is necessary because,
+ * as a prominent example, CMD sets 'Path', but not 'PATH'.
+ * Warning: not thread-safe.
+ */
+static char *getenv_cs(const char *name)
+{
+ size_t len = strlen(name);
+ int i = lookup_env(environ, name, len);
+ if (i >= 0)
+ return environ[i] + len + 1; /* skip past name and '=' */
+ return getenv(name);
+}
+
char *mingw_getenv(const char *name)
{
- char *result = getenv(name);
+ char *result = getenv_cs(name);
if (!result && !strcmp(name, "TMPDIR")) {
/* on Windows it is TMP and TEMP */
- result = getenv("TMP");
+ result = getenv_cs("TMP");
if (!result)
- result = getenv("TEMP");
+ result = getenv_cs("TEMP");
}
return result;
}
--
1.7.6.rc0.1186.gfb4fd
^ permalink raw reply related [flat|nested] 5+ messages in thread