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

Christopher Faylor
2011-10-06T14:51:28Z

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.

Otherwise, the patch is fine.

cgf