diff mbox

[6/7] Use canonicalize_file_name unconditionally

Message ID 20141128144610.GJ5042@adacore.com
State New
Headers show

Commit Message

Joel Brobecker Nov. 28, 2014, 2:46 p.m. UTC
> > Given how frequent it is for source files to be unavailable,
> > I would say that it would be useful to preserve the GetFullPathName
> > behavior for those files as well, and allow users to set breakpoints
> > without having to worry about how many backslashes GCC has been
> > using. Or said differently, I agree with your decision :-).
> >
> > I am taking a note to:
> >
> >   1. Add an extra comment to this part of the code once you've pushed
> >      your patch;
> >
> >   2. Double-check what newer versions of GCC do. At some point, perhaps
> >      we can declare a minimum GCC version that we fully support and
> >      then mark this code area for possible removal if it ever starts
> >      causing extra work.
> 
> Great, I've pushed them in.

Thank you, Yao. Attached is what I pushed for (1).

For (2), I verified that GCC 4.9 no longer suffers from this problem.
It even looks like it's generating forward slashes, even if the path
to the file given to the compiler itself follows the Windows convention
of backslashes.

gdb/ChangeLog:

        * utils.c (gdb_realpath): Rework comment about handling on
        Windows.
diff mbox

Patch

From 0fa9473ff006c0cc9e62036349ab05664b25b4c7 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Fri, 28 Nov 2014 18:37:08 +0400
Subject: [PATCH] gdb_realpath: Rework comment about handling on Windows.

Rework the comment to explain why we're still relying on GetFullPathName
even though gnulib ensures that canonicalize_file_name is now available
on all platforms, including Windows.

gdb/ChangeLog:

        * utils.c (gdb_realpath): Rework comment about handling on
        Windows.
---
 gdb/ChangeLog |  5 +++++
 gdb/utils.c   | 36 +++++++++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0e75434..a37d4e1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2014-11-28  Joel Brobecker  <brobecker@adacore.com>
+
+	* utils.c (gdb_realpath): Rework comment about handling on
+	Windows.
+
 2014-11-28  Yao Qi  <yao@codesourcery.com>
 
 	* gnulib/update-gnulib.sh (IMPORTED_GNULIB_MODULES): Add
diff --git a/gdb/utils.c b/gdb/utils.c
index b3720f6..1f5f4f4 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2868,20 +2868,38 @@  string_to_core_addr (const char *my_string)
 char *
 gdb_realpath (const char *filename)
 {
-  /* The MS Windows method.  If we don't have realpath, we assume we
-     don't have symlinks and just canonicalize to a Windows absolute
-     path.  GetFullPath converts ../ and ./ in relative paths to
-     absolute paths, filling in current drive if one is not given
-     or using the current directory of a specified drive (eg, "E:foo").
-     It also converts all forward slashes to back slashes.  */
-  /* The file system is case-insensitive but case-preserving.
-     So we do not lowercase the path.  Otherwise, we might not
-     be able to display the original casing in a given path.  */
+/* On most hosts, we rely on canonicalize_file_name to compute
+   the FILENAME's realpath.
+
+   But the situation is slightly more complex on Windows, due to some
+   versions of GCC which were reported to generate paths where
+   backlashes (the directory separator) were doubled.  For instance:
+      c:\\some\\double\\slashes\\dir
+   ... instead of ...
+      c:\some\double\slashes\dir
+   Those double-slashes were getting in the way when comparing paths,
+   for instance when trying to insert a breakpoint as follow:
+      (gdb) b c:/some/double/slashes/dir/foo.c:4
+      No source file named c:/some/double/slashes/dir/foo.c:4.
+      (gdb) b c:\some\double\slashes\dir\foo.c:4
+      No source file named c:\some\double\slashes\dir\foo.c:4.
+   To prevent this from happening, we need this function to always
+   strip those extra backslashes.  While canonicalize_file_name does
+   perform this simplification, it only works when the path is valid.
+   Since the simplification would be useful even if the path is not
+   valid (one can always set a breakpoint on a file, even if the file
+   does not exist locally), we rely instead on GetFullPathName to
+   perform the canonicalization.  */
+
 #if defined (_WIN32)
   {
     char buf[MAX_PATH];
     DWORD len = GetFullPathName (filename, MAX_PATH, buf, NULL);
 
+    /* The file system is case-insensitive but case-preserving.
+       So it is important we do not lowercase the path.  Otherwise,
+       we might not be able to display the original casing in a given
+       path.  */
     if (len > 0 && len < MAX_PATH)
       return xstrdup (buf);
   }
-- 
1.9.1