Re: [RFA] Environment variables passed to inferior by MinGW build (PR 10989)

asmwarrior
2011-09-11T14:41:17Z

On 2011-9-11 20:33, Eli Zaretskii wrote:
> There's some history to this issue. It was discussed at least twice
> before:
>
> http://sourceware.org/ml/gdb-patches/2010-05/msg00317.html
> http://sourceware.org/ml/gdb-patches/2011-04/msg00347.html
>
> The first discussion simply died because the OP never responded to
> Joel's request to clean up the patch from weird characters.
>
> As for the second discussion, upon re-reading it I can only assume
> that there was a misunderstanding of some kind, perhaps Pierre was
> talking about both the Cygwin and the native Windows builds, while
> Chris and Corinna replied only about the latter.
>
> In any case, the original problem is still there in the MinGW build of
> GDB 7.3, and it just bit me hard enough to sit down and solve it.
>
> The code in the patch below is simply taken from what was there before
> it was deleted by Corinna back in 2006, after removing from it
> everything that was Cygwin-specific. So I don't even think I can take
> credit for this code ;-)
>
> With that patch, I can add environment variables with "set
> environment", delete them with "unset environment", and the inferior
> gets the environment I meant it to have.
>
> OK to commit?
>
> 2011-09-11 Eli Zaretskii<eliz@gnu.org>
>
> * windows-nat.c (env_sort) [!__CYGWIN__]: Function restored from
> before the change on 2006-12-09.
> (windows_create_inferior) [!__CYGWIN__]: Restore code that
> generates the environment block for CreateProcessA, modulo the
> Cygwin-specific parts that are not needed here.
>
>
> === modified file 'gdb/windows-nat.c'
> --- gdb/windows-nat.c 2011-05-09 14:25:35 +0000
> +++ gdb/windows-nat.c 2011-09-11 12:26:41 +0000
> @@ -1951,6 +1951,17 @@
> *flags |= CREATE_NEW_CONSOLE;
> }
>
> +#ifndef __CYGWIN__
> +/* Function called by qsort to sort environment strings. */
> +static int
> +env_sort (const void *a, const void *b)
> +{
> + const char **p = (const char **) a;
> + const char **q = (const char **) b;
> + return strcasecmp (*p, *q);
> +}
> +#endif
> +
> /* Start an inferior windows child process and sets inferior_ptid to its pid.
> EXEC_FILE is the file to run.
> ALLARGS is a string containing the arguments to the program.
> @@ -1977,6 +1988,12 @@
> char *toexec;
> char *args;
> HANDLE tty;
> + char *w32env;
> + char *temp;
> + size_t envlen;
> + int i;
> + size_t envsize;
> + char **env;
> #endif
> PROCESS_INFORMATION pi;
> BOOL ret;
> @@ -2124,6 +2141,31 @@
> }
> }
>
> + /* CreateProcess takes the environment list as a null terminated set of
> + strings (i.e. two nulls terminate the list). */
> +
> + /* Get total size for env strings. */
> + for (envlen = 0, i = 0; in_env[i]&& *in_env[i]; i++)
> + envlen += strlen (in_env[i]) + 1;
> +
> + envsize = sizeof (in_env[0]) * (i + 1);
> + env = (char **) alloca (envsize);
> + memcpy (env, in_env, envsize);
> + /* Windows programs expect the environment block to be sorted. */
> + qsort (env, i, sizeof (char *), env_sort);
> +
> + w32env = alloca (envlen + 1);
> +
> + /* Copy env strings into new buffer. */
> + for (temp = w32env, i = 0; env[i]&& *env[i]; i++)
> + {
> + strcpy (temp, env[i]);
> + temp += strlen (temp) + 1;
> + }
> +
> + /* Final nil string to terminate new env. */
> + *temp = 0;
> +
> windows_init_thread_list ();
> ret = CreateProcessA (0,
> args, /* command line */
> @@ -2131,7 +2173,7 @@
> NULL, /* thread */
> TRUE, /* inherit handles */
> flags, /* start flags */
> - NULL, /* environment */
> + w32env, /* environment */
> NULL, /* current directory */
> &si,
> &pi);
>
>

I just apply this patch, and build a native gdb under MinGW.
I use the test code in:

http://sourceware.org/ml/gdb-patches/2010-05/msg00317.html

----------------------------------------------------------
/* env-bug.c */

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
char *env_name= "PATH";
char *env_value = getenv(env_name);

if(env_name)
printf("env-bug: %s=%s\n", env_name, env_value);
else
printf("env-bug: environment variable %s not found.\n", env_name);

env_name= "FOO";
env_value = getenv(env_name);

if(env_name)
printf("env-bug: %s=%s\n", env_name, env_value);
else
printf("env-bug: environment variable %s not found.\n", env_name);

return 0;
}
----------------------------------------------------------
(gdb) set environment FOO=bar

It works fine!

So, I think this patch should be put in trunk.

Thank you!

asmwarrior
ollydbg from codeblocks' forum