On Wed, Dec 14, 2011 at 03:07:11PM +0100, Erik Faye-Lund wrote:
> This avoids us from accidentally dropping state, possibly leading
> to unexpected behaviour.
I do think this is fine in a "be extra cautious" kind of way.
> This is especially important on Windows, where the maximum size of
> the environment is 32 kB.
But does your patch actually detect that? As Andreas pointed out, these
limits don't typically come into play at setenv time. Instead, the
environment is allocated on the heap, and then the result is passed to
exec/spawn, which will fail.
So your patch is really detecting a failure to malloc, not an overflow
of the environment size, and Windows is just as (un)likely to run out of
heap as any other platform.
You can check how your platform behaves by applying this patch:
diff --git a/git.c b/git.c
index f10e434..57f6b12 100644
--- a/git.c
+++ b/git.c
@@ -223,6 +223,16 @@ static int handle_alias(int *argcp, const char ***argv)
alias_argv[i] = (*argv)[i];
alias_argv[argc] = NULL;
+ /* make gigantic environment */
+ {
+ int len = 256 * 1024;
+ char *buf = xmalloc(len);
+ memset(buf, 'z', len);
+ buf[len-1] = '\0';
+ if (setenv("FOO", buf, 1))
+ die("setenv failed");
+ }
+
ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
if (ret >= 0) /* normal exit */
exit(ret);
and then running:
git -c alias.foo='!echo ok' foo
which yields:
fatal: cannot exec 'echo ok': Argument list too long
on Linux.
-Peff
PS I tried to come up with an invocation of git that would demonstrate
this, but it turns out it's really hard. The contents of environment
variables we set are either constants, come from the environment (so
they can't be too big already!), or come from filesystem paths. So
it's possible to overflow now, but you have to have a nearly-full
environment in the first place, and then have a long path that tips
it over the limit.
--
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