Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <pedro@palves.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2] Windows: Normalize backward slashes to forward slashes
Date: Wed, 1 Jul 2026 20:28:32 +0100	[thread overview]
Message-ID: <ff4f5fd5-b180-47ad-850e-4eda9788bf89@palves.net> (raw)
In-Reply-To: <86fr22diax.fsf@gnu.org>

On 2026-07-01 13:07, Eli Zaretskii wrote:
>> Date: Wed, 1 Jul 2026 12:28:41 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>> Still, I think the conversion is worth doing.  What the slash direction does
>> for sure affect is the argv[0] the inferior sees.  The command line we hand
>> CreateProcess is also what the child's CRT splits into argv, so without converting,
>> the inferior sees forward slashes in its own program name.  Giving the inferior
>> native separators in argv[0] seems like enough reason to convert -- i.e., not
>> framing it about what CreateProcess parses, but what the inferior itself sees
>> and parses.
> 
> I agree.
> 
>> I've made gdb and gdbserver do that, and added a comment about this.  In gdb/windows-nat.c:
>>
>>   /* Convert the executable path to backslash separators, so the
>>      inferior observes native backslashes in its own program name.  */
>>   std::string toexec_native = exec_file;
>>   std::replace (toexec_native.begin (), toexec_native.end (), '/', '\\');
>>   toexec = toexec_native.c_str ();
>>
>> and in gdbserver/win32-low.cc:
>>
>>   /* Convert the executable path to backslash separators, so the
>>      inferior observes native backslashes in its own program name.  */
>>   std::string program_native = program;
>>   std::replace (program_native.begin (), program_native.end (), '/', '\\');
>>   program = program_native.c_str ();
>>
>>
>> (Note: these are both on !Cygwin paths, as the Cygwin paths already do the path
>> conversion via cygwin_conv_path.  And BTW, Cygwin GDB already presented forward
>> slashes to users already everywhere, including in "info shared", etc.)
>>
>>
>> Having this reason written down also avoids having to try to come up with some
>> speculative comment about some old Windows behavior we're not exactly sure what it was.
>>
>>
>>>
>>>> If it's really a problem on supported Windows versions, it should be a matter of converting back to forward slashes before we
>>>> call CreateProcess.  We already do that in gdb/windows-nat.c:windows_nat_target::create_inferior, for "set cwd":
>>>>
>>>>       /* Mirror slashes on inferior's cwd.  */
>>>>       std::replace (expanded_infcwd.begin (), expanded_infcwd.end (),
>>>> 		    '/', '\\');
>>>>
>>>> we'd just need to do the same for exec_file.
>>>
>>> Yes, I think it's safer to convert to all backslashes in the argument
>>> we pass to CreateProcess.  And it will not show outside of that place,
>>> so the (very positive and welcome) effects of your changes will not be
>>> affected.
>>>
>>
>> Here's the updated patch.  Other than the tweaks mentioned above, and an updated
>> commit log, nothing else changed.
>>
>> Let me know what you think.
> 
> LGTM, thanks.  I have a couple of minor comments below.
> 
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -83,6 +83,21 @@
>>  
>>  * Support for native Thread Local Storage (TLS) variables on Windows.
>>  
>> +* GDB now normalizes backslashes to forward slashes on Windows.
>> +
>> +  E.g., depending on compiler and build system used by your project,
>> +  previously GDB could show a mix of slash styles, like for example:
>> +
>> +     C:/proj/src\main.c
>> +
>> +  GDB will now consistently show forward slashes:
>> +
>> +     C:/proj/src/main.c
>> +
>> +  This affects everywhere GDB shows a filename/dirname: source
>> +  filenames, executable filename, shared libraries, the cd/pwd
>> +  commands, etc.
> 
> Should this mention GDB/MI output?  Someone might not guess that it's
> covered by "etc.", given that the other examples are from quite
> different use cases.

How about this?

  This impacts all interpreters, including the CLI, the TUI, the
  GDB/MI interface, and the Debug Adapter Protocol, and affects
  everywhere GDB shows a filename/dirname: source filenames,
  executable filename, shared libraries, the cd/pwd commands, etc.

> 
>> +char *
>> +normalize_slashes (char *path)
>> +{
>> +  for (char *p = path; *p != '\0'; ++p)
>> +    if (*p == '\\')
>> +      *p = '/';
>> +  return path;
>> +}
> 
> Ehm... GNU Coding Standards frown on using "path" for anything but
> PATH-style directory lists.  So maybe we should use "filename" or
> somesuch, here and elsewhere.

Yeah, I had made an effort to avoid it in the NEWS entry for that reason.

For the code itself, I think that's a losing battle, the file itself
is called pathstuff.c, described at the top as:

  /* Path manipulation routines for GDB and gdbserver.

It's full of functions using "path" as I was using, and then used throughout GDB,
like:

 $ grep path ../gdbsupport/pathstuff.h  | grep extern
 extern char *normalize_slashes (char *path);
 extern gdb::unique_xmalloc_ptr<char> gdb_realpath (const char *filename);
 extern std::string gdb_realpath_keepfile (const char *filename);
 extern std::string gdb_abspath (const char *path,
 extern const char *child_path (const char *parent, const char *child);
 extern std::string path_join (gdb::array_view<const char *> paths);
 extern bool contains_dir_separator (const char *path);

Then you have standard functions/symbols like realpath, GetFullPathName, PATH_MAX, etc
(used by that file).

So "path" is pretty well established, including in POSIX and Windows APIs, and I don't
think GDB developers end up confused.

Honestly, I think using the GNU terminology confuses more people than helps.  
Recently in the DWARF committee I pointed out the GNU terminology to people, and
everyone seemed surprised and confused, and I think thought I was being weird.  :-P

If I were to have say in this aspect of the GNU coding standards, I would rather come
up with a more precise name for the PATH-style directory lists case than fight about what
does "path" mean.

> 
> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> 


  reply	other threads:[~2026-07-01 19:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29 21:24 [PATCH] " Pedro Alves
2026-06-30 11:26 ` Eli Zaretskii
2026-06-30 14:47   ` Pedro Alves
2026-06-30 15:11     ` Eli Zaretskii
2026-07-01 11:28       ` [PATCH v2] " Pedro Alves
2026-07-01 12:07         ` Eli Zaretskii
2026-07-01 19:28           ` Pedro Alves [this message]
2026-07-02  5:28             ` Eli Zaretskii
2026-07-06 12:07         ` Andrew Burgess
2026-07-06 22:35           ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ff4f5fd5-b180-47ad-850e-4eda9788bf89@palves.net \
    --to=pedro@palves.net \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox