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

[asmwarrior <asmwarrior@gmail.com>]

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