Hi,
On Sat, 2 Feb 2008, Kirill wrote:
> This is a fix, suggested by Lode Leroy, to not modify PATH in
> "our" environment, because it's actually Explorer's environment.
> See env_for_git().
>
> To simplify the code, msys_path() now never returns NULL.
The general idea is very good.
> +static void *env_for_git()
> +{
> + static char *environment = NULL;
> +
> + char *old_env, *old_path;
> + size_t old_len;
> +
> + char *src, *dst;
> +
> + if (NULL != environment)
> + return environment;
> +
> + old_env = GetEnvironmentStrings();
> +
> + /* calculate the length of the old_env */
> + old_len = 0;
> + src = old_env;
> + while (*src) {
> + size_t varlen = strlen(src) + 1;
> + old_len += varlen;
> + src += varlen;
> + };
> +
> + /* allocate new env, adjusting to the new path */
> + // 16 is the length of all added constant string
> + // (e.g. the length of \bin;\mingw\bin;)
> + environment = malloc(old_len + 2 * strlen(msys_path()) + 16);
> +
> + /* copy old environment skipping the PATH */
> + dst = environment;
> + for (src = old_env; *src; src += strlen(src)+1) {
> + if (strstr(src, "PATH=") != src) {
> + strcpy(dst, src);
> + dst += strlen(src)+1;
> + } else
> + old_path = src + 5; // +5 to skip PATH=
> + }
> +
> + /* append the new path */
> + dst += sprintf(dst, "PATH=%s\\bin;%s\\mingw\\bin;%s",
> + msys_path(), msys_path(), old_path) + 1;
> + *dst = '\0'; // terminate the environment block
Would this not be much nicer like this?
static char *environment;
if (!environment) {
char *old = GetEnvironmentStrings();
int space = 0, path_index = -1, len = 0, len2;
if (!msys_path())
return environment = old;
for (;;) {
if (!strncmp(old, "PATH=", 5))
path_index = space;
while (old[space])
space++;
if (!old[space])
break;
}
if (path_index == -1)
path_index = space;
else
len = strlen(old + path_index);
space += 2 * strlen(msys_path()) + 32;
environment = malloc(space);
memcpy(environment, old, path_index + len);
len2 = sprintf(environment + path_index,
"PATH=%s\\bin;%s\\mingw\\bin%s",
msys_path(), msys_path(), len ? ";" : "");
memcpy(environment + path_index + len2,
old + path_index + len,
space + 1 - path_index - len);
FreeEnvironmentStrings(old);
}
return environment;
}
With this (completely untested; composed in the mail program), you would
not need to change msys_path() to never return NULL...
The rest of the patch looks good to me, except for the space after "!" in
"if (! ...)".
Thanks,
Dscho
