diff mbox series

[1/1] gdbsupport: fix environment variable name handling on Windows

Message ID 20250307201957.1919-1-daniel-email@gmx.net
State New
Headers
Series [1/1] gdbsupport: fix environment variable name handling on Windows |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Daniel Starke March 7, 2025, 8:19 p.m. UTC 
  The current implementation retrieves the environment variables as it and
compares them in case sensitive manner. Windows, however, does not handle
environment variables case sensitive in most parts. See links below. As a fact,
many Windows environments define "Path" instead of "PATH".

Make the environment variable name handling case in-sensitive for MinGW, which
aims to be Windows native.

Link: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/getenv-wgetenv?view=msvc-170#remarks
Link: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-7.5#long-description
Signed-off-by: Daniel Starke <daniel-email@gmx.net>
---
 gdbsupport/environ.cc | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--
2.39.5

Comments

Tom Tromey March 7, 2025, 9:11 p.m. UTC | #1 
>>>>> "Daniel" == Daniel Starke <daniel-email@gmx.net> writes:

Daniel> The current implementation retrieves the environment variables as it and
Daniel> compares them in case sensitive manner. Windows, however, does not handle
Daniel> environment variables case sensitive in most parts. See links below. As a fact,
Daniel> many Windows environments define "Path" instead of "PATH".

Thanks for the patch.

Daniel> Make the environment variable name handling case in-sensitive
Daniel> for MinGW, which aims to be Windows native.

Makes sense but I wonder if there's a cross-debugging issue here.  Like
should Linux host with Mingw target be case-insensitive?

Daniel> +  if (strnicmp (string, var, var_len) == 0 && string[var_len] == '=')

gdb uses strncasecmp, not strnicmp

Tom
Daniel Starke March 7, 2025, 9:43 p.m. UTC | #2 
> Daniel> Make the environment variable name handling case in-sensitive
> Daniel> for MinGW, which aims to be Windows native.
>
> Makes sense but I wonder if there's a cross-debugging issue here.  Like
> should Linux host with Mingw target be case-insensitive?

No, I do not think that this makes sense.
The environment variables control the host runtime behavior. Therefore, it
should be host specific, not target specific.

> Daniel> +  if (strnicmp (string, var, var_len) == 0 && string[var_len] == '=')
>
> gdb uses strncasecmp, not strnicmp

Should I submit a new patch using strncasecmp()?

Best regards, Daniel
Tom Tromey March 8, 2025, 4:38 p.m. UTC | #3 
>>>>> "Daniel" == daniel-email  <daniel-email@gmx.net> writes:

>> Makes sense but I wonder if there's a cross-debugging issue here.  Like
>> should Linux host with Mingw target be case-insensitive?

Daniel> No, I do not think that this makes sense.
Daniel> The environment variables control the host runtime behavior.

I don't think that's correct.  "set env" is only used when creating an
inferior, not when doing host-side things like "shell" -- in fact I sent
patches in this area earlier this year.

Daniel> Should I submit a new patch using strncasecmp()?

Yes but we should perhaps resolve the other issue first.

Tom
Daniel Starke March 8, 2025, 7:04 p.m. UTC | #4 
>>> Makes sense but I wonder if there's a cross-debugging issue here.  Like
>>> should Linux host with Mingw target be case-insensitive?
> Daniel> No, I do not think that this makes sense.
> Daniel> The environment variables control the host runtime behavior.
>
> I don't think that's correct.  "set env" is only used when creating an
> inferior, not when doing host-side things like "shell" -- in fact I sent
> patches in this area earlier this year.

Interesting. The same mechanism seems is used to resolve executable paths.
The reason why I came across this was because "show" did not show me the
correct content of "PATH" on Windows (see my last patch). So do we need
to handle both cases differently? If that is the case I would need
someone with deeper knowledge of the environment variable handling in
GDB to step in for a patch.

Best regards, Daniel
diff mbox series

Patch

diff --git a/gdbsupport/environ.cc b/gdbsupport/environ.cc
index 1b6ca61ee80..75036c10d49 100644
--- a/gdbsupport/environ.cc
+++ b/gdbsupport/environ.cc
@@ -81,9 +81,14 @@  gdb_environ::clear ()
 static bool
 match_var_in_string (const char *string, const char *var, size_t var_len)
 {
+#ifdef __MINGW32__
+  /* Windows treats all environment variables case in-sensitive. */
+  if (strnicmp (string, var, var_len) == 0 && string[var_len] == '=')
+    return true;
+#else
   if (strncmp (string, var, var_len) == 0 && string[var_len] == '=')
     return true;
-
+#endif
   return false;
 }