Re: realpath() patch to fix symlinks resolution for win32

Re: realpath() patch to fix symlinks resolution for win32

[i.nixman]

[-- Attachment #1: Type: text/plain, Size: 31 bytes --]


updated patch in attachments.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lrealpath-resolve-symlinks-on-win32-bug-108350.patch --]
[-- Type: text/x-diff; name=lrealpath-resolve-symlinks-on-win32-bug-108350.patch, Size: 6191 bytes --]

diff --git a/libiberty/lrealpath.c b/libiberty/lrealpath.c
index 3c7053b0b70..8858619e854 100644
--- a/libiberty/lrealpath.c
+++ b/libiberty/lrealpath.c
@@ -68,8 +68,138 @@ 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))
+
+# define WIN32_IS_SPECIAL_NUL_FILE(ptr) \
+  (0 == strcasecmp(ptr, "NUL"))
+
+/* 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 +258,51 @@ 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);
+    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);
+
+    if ( WIN32_IS_SPECIAL_NUL_FILE(filename) ) {
+      res = get_full_path_name(filename);
+    } else {
+      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);
       }
-  }
-#endif
+    }
+
+#else
+    /* For Windows XP */
+    res = get_full_path_name(filename);
 
-  /* This system is a lost cause, just duplicate the filename.  */
-  return strdup (filename);
+#endif // _WIN32_WINNT >= 0x0600
+
+    return res;
+  }
+#endif // _WIN32
 }

Re: realpath() patch to fix symlinks resolution for win32

[Martin Liška]

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

Re: realpath() patch to fix symlinks resolution for win32

[Gerald Pfeifer]

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

Re: realpath() patch to fix symlinks resolution for win32

[NightStrike]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

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.

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong]

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.

Re: realpath() patch to fix symlinks resolution for win32

[Gerald Pfeifer]

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

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong]

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.

Re: realpath() patch to fix symlinks resolution for win32

[i.nixman]

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!

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong]

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.

Re: realpath() patch to fix symlinks resolution for win32

[Jonathan Yong]

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.

Re: realpath() patch to fix symlinks resolution for win32

[i.nixman]

hello,

could someone look at patch attached please?

https://gcc.gnu.org/pipermail/gcc-patches/2023-January/610147.html

realpath() patch to fix symlinks resolution for win32

[i.nixman]

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

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!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lrealpath-resolve-symlinks-on-win32-bug-108350.patch --]
[-- Type: text/x-diff; name=lrealpath-resolve-symlinks-on-win32-bug-108350.patch, Size: 6048 bytes --]

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
 }