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

Pierre Muller
2011-10-04T16:11:51Z

> -----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);