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

[Corinna Vinschen <vinschen at redhat dot com>]

On Oct  6 10:51, Christopher Faylor wrote:
> On Wed, Oct 05, 2011 at 02:51:57PM +0200, Corinna Vinschen wrote:
> >On Oct  5 14:08, Pierre Muller wrote:
> >>   Hi Corinna,
> >>  I didn't know about the charset problem...
> >> 
> >> > Here's my proposal, based on your patch.  I'll work on using the
> >> > yet-to-be-created new cygwin_internal call after I implemented it in
> >> > Cygwin.
> >> 
> >>   I think that your approach is OK,
> >> I checked that it does also pass all
> >> the tests in my gdb.base/testenv.exp
> >> (11 passes instead of 7 passes/4 failures for current Cygwin GDB 7.3.50 20110821cvs).
> >> 
> >> Your patch proposal thus supersedes mine.
> >> 
> >>  We just need an approval from Christopher Faylor...
> >
> >Right.  Here's a new version of the patch which uses the new
> >cygwin_internal(CW_CVT_ENV_TO_WINENV) which will be available starting
> >with the next Cygwin 1.7.10.  The patch allows to build GDB under older
> >versions of Cygwin and it will fallback to the CW_SYNC_WINENV method if
> >the cygwin_internal (CW_CVT_ENV_TO_WINENV) call returns with an error.
> >That allows to run a GDB built under 1.7.10 to run under older Cygwin
> >versions (provided all other DLL dependencies still work).  Tested under
> >Cygwin 1.7.9 and current CVS.
> >
> >
> >Corinna
> >
> >
> >        * windows-nat.c: Include wchar.h to avoid compiler warnings.
> >	Include cygwin/version.h to get version information.
> >        (clear_win32_environment): New function for Cygwin to clear out
> >        Win32 environment.
> >        (windows_create_inferior): Prepare new environment from in_env
> >        for Cygwin, too.  Use cygwin_internal (CW_CVT_ENV_TO_WINENV) call
> >	to get a Win32 copy of the desired POSIX environment if available,
> >	fall back to changing the GDB environment otherwise.
> >
> >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	5 Oct 2011 12:47:40 -0000
> >@@ -40,7 +40,9 @@
> > #include <imagehlp.h>
> > #include <psapi.h>
> > #ifdef __CYGWIN__
> >+#include <wchar.h>
> > #include <sys/cygwin.h>
> >+#include <cygwin/version.h>
> > #endif
> > #include <signal.h>
> > 
> >@@ -1963,6 +1965,28 @@ envvar_cmp (const void *a, const void *b
> > }
> > #endif
> > 
> >+#ifdef __CYGWIN__
> >+static void
> >+clear_win32_environment (char **env)
> >+{
> >+  int i;
> >+  size_t len;
> >+  wchar_t *copy = NULL, *equalpos;
> >+
> >+  for (i = 0; env[i] && *env[i]; i++)
> >+    {
> >+      len = mbstowcs (NULL, env[i], 0) + 1;
> >+      copy = (wchar_t *) xrealloc (copy, len * sizeof (wchar_t));
> >+      mbstowcs (copy, env[i], len);
> >+      equalpos = wcschr (copy, L'=');
> >+      if (equalpos)
> >+        *equalpos = L'\0';
> >+      SetEnvironmentVariableW (copy, NULL);
> >+    }
> >+  xfree (copy);
> >+}
> >+#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.
> >@@ -1980,6 +2004,8 @@ windows_create_inferior (struct target_o
> >   cygwin_buf_t *toexec;
> >   cygwin_buf_t *cygallargs;
> >   cygwin_buf_t *args;
> >+  char **old_env;
> >+  PWCHAR w32_env;
> >   size_t len;
> >   int tty;
> >   int ostdin, ostdout, ostderr;
> >@@ -2066,8 +2092,23 @@ windows_create_inferior (struct target_o
> >   strcat (args, cygallargs);
> > #endif
> > 
> >-  /* Prepare the environment vars for CreateProcess.  */
> >-  cygwin_internal (CW_SYNC_WINENV);
> >+#if CYGWIN_VERSION_API_MAJOR > 0 || CYGWIN_VERSION_API_MINOR >= 252
> 
> Why not just check if CW_CVT_ENV_TO_WINENV is defined rather than checking
> specifically for a version number?  Checking arbitrary versions like this
> should be a last resort.

The CW_foo values are not macros, but enum values.  You can't check
them for being defined.

Possible workaround is to define them twice, once as enum and once
as macro, just as Linux does or just as some Cygwin headers do,
for instance <cygwin/in.h>:

  enum
  {
    IPPROTO_IP = 0,               /* Dummy protocol for TCP               */
    IPPROTO_HOPOPTS = 0,          /* IPv6 Hop-by-Hop options              */
    [...]
  };

  #define IPPROTO_IP IPPROTO_IP
  #define IPPROTO_HOPOPTS IPPROTO_HOPOPTS
  [...]

If we do that in <sys/cygwin.h> as well, I can change the test to
#ifdef CW_CVT_ENV_TO_WINENV.  What do you think?


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat