realpath() patch to fix symlinks resolution for win32

Message ID 7c061f6c1c090362efbbcb9af9f6c758@autistici.org
State New
Headers
Series realpath() patch to fix symlinks resolution for win32 |

Commit Message

Li, Pan2 via Gcc-patches Jan. 18, 2023, 10:44 a.m. UTC
  hello again!

the final version of the path for 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350

successfully bootstraped for x86_64-mingw32 and x86_64-linux.

could anyone apply it please?



best!
  

Comments

Jonathan Yong Feb. 6, 2023, 6:40 a.m. UTC | #1
On 1/18/23 10:44, i.nixman--- via Gcc-patches wrote:
> hello again!
> 
> the final version of the path for 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> 
> successfully bootstraped for x86_64-mingw32 and x86_64-linux.
> 
> could anyone apply it please?
> 
> 
> 
> best!

Looks good to me, please supply the appropriate changelog.
  
Jonathan Yong Feb. 11, 2023, 6:32 a.m. UTC | #2
On 2/6/23 06:40, Jonathan Yong wrote:
> On 1/18/23 10:44, i.nixman--- via Gcc-patches wrote:
>> hello again!
>>
>> the final version of the path for 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
>>
>> successfully bootstraped for x86_64-mingw32 and x86_64-linux.
>>
>> could anyone apply it please?
>>
>>
>>
>> best!
> 
> Looks good to me, please supply the appropriate changelog.

Pushed to master.
  
Li, Pan2 via Gcc-patches Feb. 11, 2023, 7:28 a.m. UTC | #3
On 2023-02-11 06:32, Jonathan Yong wrote:
> On 2/6/23 06:40, Jonathan Yong wrote:
>> On 1/18/23 10:44, i.nixman--- via Gcc-patches wrote:
>>> hello again!
>>> 
>>> the final version of the path for 
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
>>> 
>>> successfully bootstraped for x86_64-mingw32 and x86_64-linux.
>>> 
>>> could anyone apply it please?
>>> 
>>> 
>>> 
>>> best!
>> 
>> Looks good to me, please supply the appropriate changelog.
> 
> Pushed to master.


Thank you Jonathan!

could you please close the corresponding BR too?:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350



best!
  
Jonathan Yong Feb. 11, 2023, 8:36 a.m. UTC | #4
On 2/11/23 07:28, i.nixman@autistici.org wrote:
> 
> Thank you Jonathan!
> 
> could you please close the corresponding BR too?:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> 
> 
> 
> best!

I can't close it, but I put a note that it has been committed.
  
Gerald Pfeifer Feb. 11, 2023, 8:52 a.m. UTC | #5
On Sat, 11 Feb 2023, Jonathan Yong via Gcc-patches wrote:
>> could you please close the corresponding BR too?:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> I can't close it, but I put a note that it has been committed.

I closed the report.

Gerald
  
Jonathan Yong Feb. 11, 2023, 1:24 p.m. UTC | #6
On 2/11/23 08:52, Gerald Pfeifer wrote:
> On Sat, 11 Feb 2023, Jonathan Yong via Gcc-patches wrote:
>>> could you please close the corresponding BR too?:
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
>> I can't close it, but I put a note that it has been committed.
> 
> I closed the report.
> 
> Gerald

Thanks.
  
NightStrike Feb. 11, 2023, 6 p.m. UTC | #7
On Sat, Feb 11, 2023 at 3:52 AM Gerald Pfeifer <gerald@pfeifer.com> wrote:
>
> On Sat, 11 Feb 2023, Jonathan Yong via Gcc-patches wrote:
> >> could you please close the corresponding BR too?:
> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> > I can't close it, but I put a note that it has been committed.
>
> I closed the report.
>
> Gerald

I would have expected the PR to have been automatically updated based on
the commit email. Any idea why that didn't happen? Not to change the state
to closed, but to add the commit information as a reply.
  
Gerald Pfeifer Feb. 11, 2023, 9:14 p.m. UTC | #8
On Sat, 11 Feb 2023, NightStrike wrote:
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
> I would have expected the PR to have been automatically updated based on
> the commit email. Any idea why that didn't happen? Not to change the state
> to closed, but to add the commit information as a reply.

I assume the fact that the PR reference was spelt as
  PR/108350
without a slash, not a blank, after "PR" may be responsible for the 
missing Bugzilla comment.

The documented format - per gcc.gnu.org/codingconventions.html - is
  PR component/12345

Martin? (By the way, where does one best have a look at those hooks?
.git/hooks in the main repository isn't it, it appears?)

Gerald
  
Martin Liška Feb. 13, 2023, 12:03 p.m. UTC | #9
On 2/11/23 22:14, Gerald Pfeifer wrote:
> On Sat, 11 Feb 2023, NightStrike wrote:
>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108350
>> I would have expected the PR to have been automatically updated based on
>> the commit email. Any idea why that didn't happen? Not to change the state
>> to closed, but to add the commit information as a reply.
> 
> I assume the fact that the PR reference was spelt as
>    PR/108350
> without a slash, not a blank, after "PR" may be responsible for the
> missing Bugzilla comment.
> 
> The documented format - per gcc.gnu.org/codingconventions.html - is
>    PR component/12345

It's likely what happens.

> 
> Martin? (By the way, where does one best have a look at those hooks?
> .git/hooks in the main repository isn't it, it appears?)

I know that server hooks (gcc.gnu.org:/home/gccadmin/hooks-bin) do contain:
email-to-bugzilla-filtered:BUGZILLA_CMD = ("/sourceware/infra/bin/email-to-bugzilla", "-G", "gcc")

thus I guess it's /sourceware/infra/bin/email-to-bugzilla what makes the filtering of commits
to bugzilla.

Martin

> 
> Gerald
  

Patch

diff --git a/libiberty/lrealpath.c b/libiberty/lrealpath.c
index 3c7053b0b70..a1ad074d00e 100644
--- a/libiberty/lrealpath.c
+++ b/libiberty/lrealpath.c
@@ -68,8 +68,135 @@  extern char *canonicalize_file_name (const char *);
   /* cygwin has realpath, so it won't get here.  */ 
 # if defined (_WIN32)
 #  define WIN32_LEAN_AND_MEAN
-#  include <windows.h> /* for GetFullPathName */
-# endif
+#  include <windows.h> /* for GetFullPathName/GetFinalPathNameByHandle/
+                          CreateFile/CloseHandle */
+#  define WIN32_REPLACE_SLASHES(_ptr, _len) \
+     for (unsigned i = 0; i != (_len); ++i) \
+       if ((_ptr)[i] == '\\') (_ptr)[i] = '/';
+
+#  define WIN32_UNC_PREFIX "//?/UNC/"
+#  define WIN32_UNC_PREFIX_LEN (sizeof(WIN32_UNC_PREFIX)-1)
+#  define WIN32_IS_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_UNC_PREFIX, WIN32_UNC_PREFIX_LEN))
+
+#  define WIN32_NON_UNC_PREFIX "//?/"
+#  define WIN32_NON_UNC_PREFIX_LEN (sizeof(WIN32_NON_UNC_PREFIX)-1)
+#  define WIN32_IS_NON_UNC_PREFIX(ptr) \
+  (0 == memcmp(ptr, WIN32_NON_UNC_PREFIX, WIN32_NON_UNC_PREFIX_LEN))
+
+/* Get full path name without symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_full_path_name(const char *filename) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+     from the man: `If the lpBuffer buffer is too small to contain
+     the path, the return value is the size, in TCHARs, of the buffer
+     that is required to hold the path _and_the_terminating_null_character_`
+  */
+  len = GetFullPathName(filename, 0, NULL, NULL);
+
+  if ( len == 0 )
+    return strdup(filename);
+
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFullPathName(filename, len, buf, NULL);
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+    ? (WIN32_UNC_PREFIX_LEN - 2)
+    : WIN32_IS_NON_UNC_PREFIX(buf)
+      ? WIN32_NON_UNC_PREFIX_LEN
+      : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+    ptr[0] = '/';
+    ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# if _WIN32_WINNT >= 0x0600
+
+/* Get full path name WITH symlinks resolution.
+   It also converts all forward slashes to back slashes.
+*/
+char* get_final_path_name(HANDLE fh) {
+  DWORD len;
+  char *buf, *ptr, *res;
+
+  /* determining the required buffer size.
+     from the  man: `If the function fails because lpszFilePath is too
+     small to hold the string plus the terminating null character,
+     the return value is the required buffer size, in TCHARs. This
+     value _includes_the_size_of_the_terminating_null_character_`.
+     but in my testcase I have path with 26 chars, the function
+     returns 26 also, ie without the trailing zero-char...
+  */
+  len = GetFinalPathNameByHandle(
+     fh
+    ,NULL
+    ,0
+    ,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+
+  if ( len == 0 )
+    return NULL;
+
+  len += 1; /* for zero-char */
+  buf = (char *)malloc(len);
+
+  /* no point to check the result again */
+  len = GetFinalPathNameByHandle(
+     fh
+    ,buf
+    ,len
+    ,FILE_NAME_NORMALIZED | VOLUME_NAME_DOS
+  );
+  buf[len] = 0;
+
+  /* replace slashes */
+  WIN32_REPLACE_SLASHES(buf, len);
+
+  /* calculate offset based on prefix type */
+  len = WIN32_IS_UNC_PREFIX(buf)
+    ? (WIN32_UNC_PREFIX_LEN - 2)
+    : WIN32_IS_NON_UNC_PREFIX(buf)
+      ? WIN32_NON_UNC_PREFIX_LEN
+      : 0
+  ;
+
+  ptr = buf + len;
+  if ( WIN32_IS_UNC_PREFIX(buf) ) {
+    ptr[0] = '/';
+    ptr[1] = '/';
+  }
+
+  res = strdup(ptr);
+
+  free(buf);
+
+  return res;
+}
+
+# endif // _WIN32_WINNT >= 0x0600
+
+# endif // _WIN32
 #endif
 
 char *
@@ -128,30 +255,52 @@  lrealpath (const char *filename)
   }
 #endif
 
-  /* 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 MS Windows method */
 #if defined (_WIN32)
   {
-    char buf[MAX_PATH];
-    char* basename;
-    DWORD len = GetFullPathName (filename, MAX_PATH, buf, &basename);
-    if (len == 0 || len > MAX_PATH - 1)
-      return strdup (filename);
-    else
-      {
-	/* The file system is case-preserving but case-insensitive,
-	   Canonicalize to lowercase, using the codepage associated
-	   with the process locale.  */
-        CharLowerBuff (buf, len);
-        return strdup (buf);
-      }
-  }
-#endif
+    char *res;
+
+    /* For Windows Vista and greater */
+#if _WIN32_WINNT >= 0x0600
+
+    /* For some reason the function receives just empty `filename`, but not NULL.
+       What should we do in that case?
+       According to `strdup()` implementation
+         (https://elixir.bootlin.com/glibc/latest/source/string/strdup.c)
+       it will alloc 1 byte even for empty but non NULL string.
+       OK, will use `strdup()` for that case.
+    */
+    if ( 0 == strlen(filename) )
+      return strdup(filename);
+
+    HANDLE fh = CreateFile(
+       filename
+      ,FILE_READ_ATTRIBUTES
+      ,FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE
+      ,NULL
+      ,OPEN_EXISTING
+      ,FILE_FLAG_BACKUP_SEMANTICS
+      ,NULL
+    );
+
+    if ( fh == INVALID_HANDLE_VALUE ) {
+      res = get_full_path_name(filename);
+    } else {
+      res = get_final_path_name(fh);
+      CloseHandle(fh);
 
-  /* This system is a lost cause, just duplicate the filename.  */
-  return strdup (filename);
+      if ( !res )
+        res = get_full_path_name(filename);
+    }
+
+#else
+
+    /* For Windows XP */
+    res = get_full_path_name(filename);
+
+#endif // _WIN32_WINNT >= 0x0600
+
+    return res;
+  }
+#endif // _WIN32
 }