RE: [RFA] testsuite: Add a test for passing of environment variables to inferior

["Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Corinna Vinschen
> Envoyà : mardi 4 octobre 2011 17:13
> Ã : gdb-patches@sourceware.org
> Objet : Re: [RFA] testsuite: Add a test for passing of environment variables
> to inferior
> 
> On Oct  4 16:49, Pierre Muller wrote:
> > > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> > > owner@sourceware.org] De la part de Corinna Vinschen
> > > Yes, that would be necessary.  I'm wondering if we can't just utilze the
> > > global environ variable for this and spare us all the hassle.  Something
> > > along these lines:
> > >
> > >   char **old_env = environ;
> > >   environ = in_env;
> > >   cygwin_internal (CW_SYNC_WINENV);
> > >   CreateProcessW (NULL environment pointer);
> > >   environ = old_env;
> >
> >   I checked it out, but it still
> > fails for the last test, the missing TEST_GDB_VAR1
> > variable doesn't get removed from GDB environment list
> > apparently (This is probably related to the internals
> > of cygwin_internal (CW_SYNC_WINENV), no?
> 
> Right.  The problem is that there's no simple Win32 call to set (or
> clear) the Windows environment in one go.  So CW_SYNC_WINENV only sets
> but never unsets because that would require to check each Windows
> variable if it exists in the POSIX env or not.  That was more effort
> than seemed to make sense at the time.
> 
> >   Manually resetting all variables should help...
> 
> Yes, but that's far from elegant.  I don't mean your patch, but the fact
> that it seems to be necessary at all.  Maybe we should introduce some
> new cygwin_internal call, which produces a Win32 environment from a
> POSIX environment without affecting the Win32 environment of the calling
> process:
> 
>   LPWCH out_env = (LPWCH) cygwin_internal (CW_CREATE_WIN32_ENV, in_env);
>   CreateProcess (..., CREATE_UNICODE_ENVIRONMENT, ..., out_env, ...);
>   free (new_env);
> 
> But that would only work for newer Cygwin releases, so that's just
> a future option.
> 
> > The patch below does not generate any failure on my test,
> > but I am still not sure what happens if you try to
> > remove an environment variable that was set when GDB started...
> 
> GDB should not be affected.  Its POSIX environment is still intact and
> that's all what's used when interacting with the underlying Cygwin POSIX
> calls.  The only variable which should have an effect is $PATH, and that's
> restored by the final cygwin_internal(CW_SYNC_WINENV) call.  So, AFAICS,
> your patch should be fine.

  Nor completely yet,
here is an update that unsets all environment
variables both in environ list before calling
CreateProcess and in in_env list after...

  This passes the new updated test I just sent.



2011-10-04  Pierre Muller  <muller@ics.u-strasbg.fr>

	* windows-nat.c (windows_create_inferior): Handle in_env
	array for Cygwin GDB.
	* testsuite/gdb.base/setshow.exp (gdb_test): Add test to check
	that 'unset environment' works.


Index: windows-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/windows-nat.c,v
retrieving revision 1.219
diff -u -p -r1.219 windows-nat.c
--- windows-nat.c	28 Sep 2011 09:07:54 -0000	1.219
+++ windows-nat.c	4 Oct 2011 16:05:36 -0000
@@ -1980,8 +1980,9 @@ windows_create_inferior (struct target_o
   cygwin_buf_t *toexec;
   cygwin_buf_t *cygallargs;
   cygwin_buf_t *args;
+  char **old_env;
   size_t len;
-  int tty;
+  int tty, i;
   int ostdin, ostdout, ostderr;
 #else
   char real_path[__PMAX];
@@ -2066,6 +2067,19 @@ windows_create_inferior (struct target_o
   strcat (args, cygallargs);
 #endif
 
+  /* Reset all environment variables to avoid leftover on next run. */
+  for (i = 0; environ[i] && *environ[i]; i++)
+    {
+      char *equalpos;
+      char *copy = alloca (strlen(environ[i]) + 1);
+      strcpy (copy, environ[i]);
+      equalpos = strchr (copy, '=');
+      if (equalpos)
+	*equalpos = '\0';
+      SetEnvironmentVariableA (copy, NULL);
+    }
+  old_env = environ;
+  environ = in_env;
   /* Prepare the environment vars for CreateProcess.  */
   cygwin_internal (CW_SYNC_WINENV);
 
@@ -2101,6 +2115,20 @@ windows_create_inferior (struct target_o
 		       NULL,	/* current directory */
 		       &si,
 		       &pi);
+  /* Reset all environment variables to avoid leftover on next run. */
+  for (i = 0; in_env[i] && *in_env[i]; i++)
+    {
+      char *equalpos = strchr (in_env[i], '=');
+      if (equalpos)
+	*equalpos = '\0';
+      SetEnvironmentVariableA (in_env[i], NULL);
+      if (equalpos)
+	*equalpos = '=';
+    }
+  /* Restore normal GDB environment variables.  */
+  environ = old_env;
+  cygwin_internal (CW_SYNC_WINENV);
+
   if (tty >= 0)
     {
       close (tty);