On Sun, 21 Sept 2025, 04:10 Adam Wood, <[email protected]> wrote:

>
> Thank you so much for the review! I'll get to work incorporating it into
> v3 of the patch shortly.
>
> On Sat, Sep 20, 2025 at 2:51 PM Jonathan Wakely <[email protected]>
> wrote:
>
>> On Mon, 11 Aug 2025 at 10:25 -0600, Adam Wood wrote:
>> >I am reposting this patch because I haven't gotten any feedback since I
>> >originally posted it a few weeks ago.
>>
>> Thanks for working on this. I can't review the uses of the Windows
>> APIs for correctness as I know nothing about them, but this seems to
>> cover what's needed.
>>
>> Some comments and questions below ...
>>
>> >Changes in v2: Wrapped a Windows specific function in an #ifdef.
>> >
>> >This patch adds symlink support on Windows.
>> >I tested it on x86_64-w64-mingw32 with msys2.
>> >A few notes about this patch:
>> >
>> >1. Symlinks can only be created while running as admin, or if Developer
>> Mode
>> >   is enabled on Windows 10/11.
>> >2. Symlinks on Windows are either file symlinks or directory symlinks.
>> >   I decided that fs::create_symlink should only be used for creating
>> file
>> >   symlinks, unlike the implementation on Linux. I am not sure if this
>> >   was the correct decision.
>>
>> Yes, that seems right to me.
>>
>> >3. Windows seems to process dotdots in paths before it resolves symlinks.
>> >   This results in errors.
>> >   For example, in test03 for fs::canonical, there is a directory setup
>> with
>> >   dir, dir/foo, and dir/bar. dir/foo contains a symlink, baz,
>> >   which points to ../bar. The test checks that
>> >   fs::canonical(dir/foo/baz/../bar) equals dir/foo/../bar/../bar or
>> dir/bar.
>> >   However, Windows interprets the path as dir/foo/bar.
>> >   This is the reason my patch treats paths with a dotdot carefully
>> >   in fs::absolute and fs::canonical.
>> >
>> >libstdc++-v3/Changelog:
>> >
>> >       * src/c++17/fs_ops.cc:
>> >       Include <winioctl.h> for FSCTL_GET_REPARSE_POINT.
>> >       (path_starts_with_dotdot): New helper function for fs::absolute.
>> >       (fs::absolute): Call GetFullPathNameW on part of path before ".."
>> >       and then append the rest to the result.
>> >       (fs::canonical): Don't use lexically_normal in call to
>> fs::absolute.
>> >       Don't check if path exists if it contains "..".
>> >       Check that component of path exists if it isn't empty or "." or
>> "..".
>> >       (windows_create_symlink): New helper function for
>> fs::create_symlink
>> >       and fs::create_directory_symlink.
>> >       (fs::create_directory_symlink): Call windows_create_symlink on
>> Windows.
>> >       (fs::create_symlink): Call windows_create_symlink on Windows.
>> >       (auto_win_file_handle::auto_win_file_handle): Add follow_symlink
>> >       parameter to control whether the handle should open the symlink
>> >       or the target. The parameter defaults to opening the target.
>> >       (windows_read_symlink_handle): New helper function
>> >       for fs::read_symlink.
>> >       (fs::read_symlink): Call windows_read_symlink_handle on Windows.
>> >       (fs::remove): Call RemoveDirectoryW only for directories, and
>> >       DeleteFileW for regular files, but attempt both for symlinks.
>> >       (fs::remove_all): Return immediately if path is empty.
>> >       Check if path points to a symlink, and if so, remove the
>> >       symlink using fs::remove.
>> >       * src/filesystem/ops-common.h:
>> >       Define S_IFLNK and S_ISLNK.
>> >       (__gnu_posix::__open_for_stat): New helper function for stat and
>> lstat.
>> >       (__gnu_posix::__is_handle_symlink): New helper function for
>> >       stat, lstat, and fs::read_symlink.
>> >       (__gnu_posix::__stat_windows): New helper function for stat and
>> lstat.
>> >       (__gnu_posix::stat, __gnu_posix::lstat): Use __stat_windows to
>> properly
>> >       follow or not follow symlinks, and check if file is a symlink.
>> >       * testsuite/27_io/filesystem/operations/canonical.cc (test03):
>> >       Use fs::create_directory_symlink instead of fs::create_symlink.
>> >       * testsuite/27_io/filesystem/operations/copy.cc (test02):
>> >       Create a symlink to temporary file instead of ".".
>> >       Use fs::exists(symlink_status()) instead of fs::exists for
>> symlinks.
>> >       * testsuite/27_io/filesystem/operations/weakly_canonical.cc
>> (test01):
>> >       Use fs::create_directory_symlink instead of fs::create_symlink.
>> >       * testsuite/util/testsuite_fs.h: Do not define NO_SYMLINKS on
>> Windows.
>> >---
>> > libstdc++-v3/src/c++17/fs_ops.cc              | 291 +++++++++++++++++-
>> > libstdc++-v3/src/filesystem/ops-common.h      |  75 ++++-
>> > .../27_io/filesystem/operations/canonical.cc  |   2 +-
>> > .../27_io/filesystem/operations/copy.cc       |   7 +-
>> > .../filesystem/operations/weakly_canonical.cc |   2 +-
>> > libstdc++-v3/testsuite/util/testsuite_fs.h    |   2 +
>> > 6 files changed, 359 insertions(+), 20 deletions(-)
>> >
>> >diff --git a/libstdc++-v3/src/c++17/fs_ops.cc
>> b/libstdc++-v3/src/c++17/fs_ops.cc
>> >index 4f188153ae3..a8a2f19d1c2 100644
>> >--- a/libstdc++-v3/src/c++17/fs_ops.cc
>> >+++ b/libstdc++-v3/src/c++17/fs_ops.cc
>> >@@ -56,6 +56,7 @@
>> > #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> > # define WIN32_LEAN_AND_MEAN
>> > # include <windows.h>
>> >+# include <winioctl.h> // FSCTL_GET_REPARSE_POINT
>> > #endif
>> >
>> > #define _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM namespace filesystem {
>> >@@ -78,6 +79,23 @@ fs::absolute(const path& p)
>> >   return ret;
>> > }
>> >
>> >+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+namespace
>> >+{
>> >+  bool
>> >+  path_starts_with_dotdot(std::wstring_view p)
>> >+  {
>> >+    if (p.size() < 2)
>> >+      return false;
>> >+    if (p[0] == L'.' && p[1] == L'.')
>> >+      return true;
>>
>> Is "..." a valid filename? Because this function would say it starts
>> with dotdot, but it's not the same as something like "../foo".
>>
>> >+    if (p.size() < 3 || !(p[0] == L'/' || p[0] == L'\\'))
>> >+      return false;
>>
>> What does "/.." mean on Windows?
>>
>> Does the same issue with "/..." exist here?
>>
>> Would it be better to check if the first non-root-directory component
>> of the path is equal to ".." rather than doing string comparisons?
>>
>
> Yeah, I checked and although "..." is not allowed, "..abc", "abc..def",
> etc. are allowed.
> In v3, I'll iterate over the components and check if each component equals
> ".." instead.
>
>
>> >+    return p[1] == L'.' && p[2] == L'.';
>> >+  }
>> >+}
>> >+#endif
>> >+
>> > fs::path
>> > fs::absolute(const path& p, error_code& ec)
>> > {
>> >@@ -97,6 +115,8 @@ fs::absolute(const path& p, error_code& ec)
>> > #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >   // s must remain null-terminated
>> >   wstring_view s = p.native();
>> >+  path after_dotdot_p;
>> >+  path before_dotdot_p;
>> >
>> >   if (p.has_root_directory()) // implies !p.has_root_name()
>> >     {
>> >@@ -108,6 +128,25 @@ fs::absolute(const path& p, error_code& ec)
>> >       s.remove_prefix(std::min(s.length(), pos) - 1);
>> >     }
>> >
>> >+
>> >+  // GetFullPathNameW does not work correctly with a .. right after a
>> symlink,
>> >+  // so if we have a .. in the path, run GetFullPathNameW on the part
>> before
>> >+  // the dotdot, and then append the rest.
>> >+  if (!path_starts_with_dotdot(s) && s.find(L"..") !=
>> wstring_view::npos)
>> >+    {
>> >+      bool before_dotdot = true;
>> >+      for (const auto& component : p)
>> >+      {
>> >+        if (before_dotdot && !wcscmp(component.c_str(), L".."))
>>
>> Couldn't this just use component.native() == L".." instead of wcscmp?
>>
>> >+              before_dotdot = false;
>> >+        if (before_dotdot)
>> >+          before_dotdot_p /= component;
>> >+        else
>> >+          after_dotdot_p /= component;
>> >+      }
>> >+      s = before_dotdot_p.native();
>> >+    }
>> >+
>> >   uint32_t len = 1024;
>> >   wstring buf;
>> >   do
>> >@@ -123,6 +162,9 @@ fs::absolute(const path& p, error_code& ec)
>> >     ec = __last_system_error();
>> >   else
>> >     ret = std::move(buf);
>> >+
>> >+  if (!after_dotdot_p.empty())
>> >+    ret /= after_dotdot_p;
>> > #else
>> >   ret = current_path(ec);
>> >   ret /= p;
>> >@@ -162,11 +204,7 @@ fs::path
>> > fs::canonical(const path& p, error_code& ec)
>> > {
>> >   path result;
>> >-#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >-  const path pa = absolute(p.lexically_normal(), ec);
>> >-#else
>> >   const path pa = absolute(p, ec);
>> >-#endif
>> >   if (ec)
>> >     return result;
>> >
>> >@@ -192,7 +230,17 @@ fs::canonical(const path& p, error_code& ec)
>> >     }
>> > #endif
>> >
>> >+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+  // Windows handles relative paths after symlinks incorrectly.
>> >+  // If we have a .. after a symlink, the .. will be cancelled before
>> the
>> >+  // symlink is resolved.  For example, if we have baz -> ../bar,
>> >+  // and we have the path dir/foo/baz/../bar, Linux would interpret
>> this as
>> >+  // dir/foo/../bar/../bar = dir/bar, but Windows thinks the path is
>> >+  // dir/foo/bar.  This leads to possible false negatives using exists.
>> >+  if (pa.native().find(L"..") == std::wstring::npos && !exists(pa, ec))
>>
>> Again, is "..." or "abc..def" a valid path component?
>>
>> Do we want a "has dotdot component" function, which checks for an
>> actual dotdot component, not just ".." anywhere in the path?
>>
>> >+#else
>> >   if (!exists(pa, ec))
>> >+#endif
>> >     {
>> >       if (!ec)
>> >       ec = make_error_code(std::errc::no_such_file_or_directory);
>> >@@ -234,7 +282,15 @@ fs::canonical(const path& p, error_code& ec)
>> >       {
>> >         result /= f;
>> >
>> >-        if (is_symlink(result, ec))
>> >+        auto st = symlink_status(result, ec);
>> >+        if (!exists(st))
>> >+          {
>> >+            if (!ec)
>> >+              ec.assign(ENOENT, std::generic_category());
>> >+            result.clear();
>> >+            return result;
>> >+          }
>>
>> This part of the code is used on all targets, not just Windows. Is it
>> correct for all targets? Didn't we already check earlier in this
>> function that the target exists?
>>
>
> The call to exists is only necessary on Windows, so I'll wrap that in an
> ifdef in the next version of the patch.
>
> But the exists call here on Windows is necessary because we can't trust
> the result of exists on variable pa, and I'll explain why.
> On Linux, the OS filesystem functions will resolve symlinks and then
> resolve dotdots.
> On Windows, dotdots are resolved first, and then symlinks.
>
> Let's say we have the directory dir which contains the directory bar, and
> the directory foo.
> foo contains the symlink baz which points to ../bar
> If we had the path dir/foo/baz/../bar, Linux would first replace symlinks
> with targets to get dir/foo/../bar/../bar.
> Then the dotdots are dealt with giving dir/bar. dir/bar exists.
>
> On Windows, the same path is dealt with by first removing dotdots, giving
> dir/foo/bar.
> dir/foo/bar does not exist, but the intended canonical path, dir/bar, does.
> Therefore, using exists can give incorrect results if a symlink is next to
> a dotdot, and so we have to check
> that the path exists in this inner section of the function where we have
> dealt with the dotdots and symlinks.
>

Got it, thanks.


>
>> >+        if (is_symlink(st))
>> >           {
>> >             path link = read_symlink(result, ec);
>> >             if (!ec)
>> >@@ -644,6 +700,44 @@ fs::create_directory(const path& p, const path&
>> attributes,
>> > #endif
>> > }
>> >
>> >+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+namespace
>> >+{
>> >+  void
>> >+  windows_create_symlink(const fs::path& to, const fs::path&
>> new_symlink,
>> >+                       const fs::file_type target_type,
>> >+                       std::error_code& ec) noexcept
>> >+  {
>> >+    auto symlink_flags = SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE;
>> >+    if (target_type == fs::file_type::directory)
>> >+      symlink_flags |= SYMBOLIC_LINK_FLAG_DIRECTORY;
>> >+    // Windows can't handle relative symlinks with non-preferred
>> slashes.
>> >+    // Creating the symlink will succeed, but the symlink won't resolve
>> >+    // correctly in later operations.
>> >+    const fs::path* preferred_to = &to;
>> >+    fs::path to2;
>> >+    if (to.native().find(L'/') != std::string::npos)
>>
>> Can we just use to.make_preferred() unconditionally? It already does
>> nothing if find(L'/') returns npos, and it never throws in our
>> implementation.
>>
>
> Since I am working with const references, I have to make a copy of to
> which could throw.
> I could create the copy and call make_preferred unconditionally, but I
> would still have a try/catch block because
> the create symlink function overloads with an error code are marked
> noexcept.
>

Doh, I missed that it's the copy that can throw not the make_preferred()
call. I agree that avoiding the copy when it's not needed is better.



>
>>
>> >+      {
>> >+      __try
>> >+        {
>> >+          to2 = to;
>> >+          to2.make_preferred();
>> >+          preferred_to = &to2;
>> >+        }
>> >+      __catch (const std::bad_alloc&)
>> >+        {
>> >+          ec = std::make_error_code(std::errc::not_enough_memory);
>> >+          return;
>> >+        }
>> >+      }
>> >+    if (CreateSymbolicLinkW(new_symlink.c_str(), preferred_to->c_str(),
>> >+                          symlink_flags))
>> >+      ec.clear();
>> >+    else
>> >+      ec = std::__last_system_error();
>> >+  }
>> >+}
>> >+#endif
>> >
>> > void
>> > fs::create_directory_symlink(const path& to, const path& new_symlink)
>> >@@ -660,7 +754,7 @@ fs::create_directory_symlink(const path& to, const
>> path& new_symlink,
>> >                            error_code& ec) noexcept
>> > {
>> > #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >-  ec = std::make_error_code(std::errc::function_not_supported);
>> >+  windows_create_symlink(to, new_symlink, file_type::directory, ec);
>> > #else
>> >   create_symlink(to, new_symlink, ec);
>> > #endif
>> >@@ -715,6 +809,8 @@ fs::create_symlink(const path& to, const path&
>> new_symlink,
>> >     ec.assign(errno, std::generic_category());
>> >   else
>> >     ec.clear();
>> >+#elif _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+  windows_create_symlink(to, new_symlink, file_type::regular, ec);
>> > #else
>> >   ec = std::make_error_code(std::errc::function_not_supported);
>> > #endif
>> >@@ -829,10 +925,14 @@ namespace
>> >   struct auto_win_file_handle
>> >   {
>> >     explicit
>> >-    auto_win_file_handle(const wchar_t* p, std::error_code& ec) noexcept
>> >+    auto_win_file_handle(const wchar_t* p, std::error_code& ec,
>> >+                       const bool follow_symlink = true) noexcept
>> >     : handle(CreateFileW(p, 0,
>> >                        FILE_SHARE_DELETE | FILE_SHARE_READ |
>> FILE_SHARE_WRITE,
>> >-                       0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS,
>> 0)),
>> >+                       0, OPEN_EXISTING,
>> >+                       (FILE_FLAG_BACKUP_SEMANTICS
>> >+                        | (FILE_FLAG_OPEN_REPARSE_POINT *
>> !follow_symlink)),
>>
>> Can we just do (follow_symlink ? 0 : FILE_FLAG_OPEN_REPARSE_POINT)
>>
>>
> Yeah, thats definitely better.
>
>
>> >+                       0)),
>> >       ec(ec)
>> >     {
>> >       if (handle == INVALID_HANDLE_VALUE)
>> >@@ -1193,6 +1293,100 @@ fs::proximate(const path& p, const path& base,
>> error_code& ec)
>> >   return result;
>> > }
>> >
>> >+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+namespace
>> >+{
>> >+  // This would be defined in <ntifs.h> if it were included.
>>
>> Can we include it, or is it a kernel header or something?
>>
>
> I tried, but it wouldn't compile. That was in July though, so I'll try
> again.
>
>
>>
>> >+  typedef struct _REPARSE_DATA_BUFFER {
>> >+    ULONG  ReparseTag;
>> >+    USHORT ReparseDataLength;
>> >+    USHORT Reserved;
>> >+    union {
>> >+      struct {
>> >+      USHORT SubstituteNameOffset;
>> >+      USHORT SubstituteNameLength;
>> >+      USHORT PrintNameOffset;
>> >+      USHORT PrintNameLength;
>> >+      ULONG  Flags;
>> >+      WCHAR  PathBuffer[1];
>> >+      } SymbolicLinkReparseBuffer;
>> >+      struct {
>> >+      USHORT SubstituteNameOffset;
>> >+      USHORT SubstituteNameLength;
>> >+      USHORT PrintNameOffset;
>> >+      USHORT PrintNameLength;
>> >+      WCHAR  PathBuffer[1];
>> >+      } MountPointReparseBuffer;
>> >+      struct {
>> >+      UCHAR DataBuffer[1];
>> >+      } GenericReparseBuffer;
>> >+    } DUMMYUNIONNAME;
>> >+  } REPARSE_DATA_BUFFER, *PREPARSE_DATA_BUFFER;
>> >+
>> >+  void
>> >+  windows_read_symlink_handle(auto_win_file_handle& link_handle,
>> >+                            std::error_code& ec,
>> >+                            fs::path& result)
>> >+  {
>> >+    REPARSE_DATA_BUFFER * reparse_buffer = nullptr;
>>
>> Should this use the PREPARSE_DATA_BUFFER type?
>>
>
> That would be more idiomatic, I suppose.
>
>
>>
>> >+    std::unique_ptr<char[]> big_buffer;
>> >+
>> >+    // Allocate enough memory on the stack to get the reparse data
>> >+    // plus a 260 character path.  Should be sufficient in most cases.
>> >+    // Allocate an extra wchar_t to ensure we can manually null
>> terminate.
>> >+    static constexpr size_t small_buffer_size =
>> sizeof(REPARSE_DATA_BUFFER)
>> >+                                              + 260 * sizeof(wchar_t);
>> >+    char small_buffer[small_buffer_size + sizeof(wchar_t)];
>> >+    reparse_buffer =
>> reinterpret_cast<REPARSE_DATA_BUFFER*>(&small_buffer[0]);
>> >+    long unsigned int bytes_returned, big_buffer_size;
>> >+
>> >+    // Attempt to get the reparse data with the buffer on the stack
>> >+    // before allocating the exact amount needed on the heap.
>> >+    bool got_reparse_data = DeviceIoControl(link_handle.handle,
>> >+                                          FSCTL_GET_REPARSE_POINT,
>> >+                                          nullptr, 0,
>> >+                                          &small_buffer,
>> small_buffer_size,
>> >+                                          &bytes_returned, nullptr);
>> >+
>> >+    int last_error = GetLastError();
>> >+    if (!got_reparse_data && last_error == ERROR_MORE_DATA)
>> >+      {
>> >+      big_buffer_size = bytes_returned;
>> >+      big_buffer.reset(new char[big_buffer_size + sizeof(wchar_t)]);
>> >+      got_reparse_data = DeviceIoControl(link_handle.handle,
>> >+                                         FSCTL_GET_REPARSE_POINT,
>> >+                                         nullptr, 0,
>> >+                                         big_buffer.get(),
>> big_buffer_size,
>> >+                                         &bytes_returned, nullptr);
>> >+      if (!got_reparse_data)
>> >+        {
>> >+          ec = std::__last_system_error();
>> >+          return;
>> >+        }
>> >+
>> >+      reparse_buffer
>> >+        = reinterpret_cast<REPARSE_DATA_BUFFER*>(big_buffer.get());
>> >+
>> >+      }
>> >+    else
>> >+      {
>> >+      if (!got_reparse_data)
>> >+        {
>> >+          ec = std::__last_system_error();
>> >+          return;
>> >+        }
>> >+      }
>> >+
>> >+    ec.clear();
>> >+    auto& symlink_buffer = reparse_buffer->SymbolicLinkReparseBuffer;
>> >+    wchar_t* target_name = &symlink_buffer.PathBuffer[0];
>> >+    target_name += symlink_buffer.PrintNameOffset / sizeof(wchar_t);
>> >+    target_name[symlink_buffer.PrintNameLength / sizeof(wchar_t)] =
>> L'\0';
>> >+    result = target_name;
>> >+  }
>> >+};
>> >+#endif // _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+
>> > fs::path
>> > fs::read_symlink(const path& p)
>> > {
>> >@@ -1248,6 +1442,25 @@ fs::path fs::read_symlink(const path& p,
>> error_code& ec)
>> >       bufsz *= 2;
>> >     }
>> >   while (true);
>> >+#elif _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+  auto_win_file_handle link_handle(p.c_str(), ec, false);
>> >+  if (!link_handle)
>> >+    return result;
>> >+
>> >+  int is_symlink = posix::__is_handle_symlink(link_handle.handle);
>> >+  if (is_symlink == -1)
>> >+    {
>> >+      ec = __last_system_error();
>> >+      return result;
>> >+    }
>> >+
>> >+  if (!is_symlink)
>> >+    {
>> >+      ec.assign(EINVAL, std::generic_category());
>> >+      return result;
>> >+    }
>> >+
>> >+  windows_read_symlink_handle(link_handle, ec, result);
>> > #else
>> >   ec = std::make_error_code(std::errc::function_not_supported);
>> > #endif
>> >@@ -1291,8 +1504,14 @@ fs::remove(const path& p, error_code& ec) noexcept
>> >   auto st = symlink_status(p, ec);
>> >   if (exists(st))
>> >     {
>> >-      if ((is_directory(p, ec) && RemoveDirectoryW(p.c_str()))
>> >-        || DeleteFileW(p.c_str()))
>> >+      if ((is_directory(st) || is_symlink(st))
>> >+        && RemoveDirectoryW(p.c_str()))
>> >+      {
>> >+        ec.clear();
>> >+        return true;
>> >+      }
>> >+      else if ((is_regular_file(st) || is_symlink(st))
>> >+             && DeleteFileW(p.c_str()))
>> >       {
>> >         ec.clear();
>> >         return true;
>> >@@ -1320,6 +1539,30 @@ std::uintmax_t
>> > fs::remove_all(const path& p)
>> > {
>> >   error_code ec;
>> >+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+  if (p.empty())
>> >+    return 0;
>> >+  // The current opendir implementation on Windows always follows an
>> initial
>> >+  // symlink.  Therefore, if remove_all is called on a symlink,
>> >+  // the target is removed.  Call remove if we have a symlink.
>> >+  auto p_status = symlink_status(p, ec);
>> >+  if (!exists(p_status))
>> >+    {
>> >+      int err = ec.default_error_condition().value();
>> >+      bool not_found = !ec || is_not_found_errno(err);
>> >+      if (!not_found)
>> >+      _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all",
>> >+                                               p, ec));
>> >+      return 0;
>> >+    }
>> >+  if (is_symlink(p_status))
>> >+    {
>> >+      if (!remove(p, ec))
>> >+      _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot remove all",
>> >+                                               p, ec));
>> >+      return 1;
>> >+    }
>> >+#endif
>> >   uintmax_t count = 0;
>> >   recursive_directory_iterator dir(p, directory_options{64|128}, ec);
>> >   switch (ec.value()) // N.B. assumes ec.category() ==
>> std::generic_category()
>> >@@ -1363,6 +1606,34 @@ fs::remove_all(const path& p)
>> > std::uintmax_t
>> > fs::remove_all(const path& p, error_code& ec)
>> > {
>> >+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> >+  if (p.empty())
>> >+    {
>> >+      ec.clear();
>> >+      return 0;
>> >+    }
>> >+  // The current opendir implementation on Windows always follows an
>> initial
>> >+  // symlink.  Therefore, if remove_all is called on a symlink,
>> >+  // the target is removed.  Call remove if we have a symlink, then.
>>
>> This differs from the comment above by just ", then" at the end - is
>> that significant, or something that was meant to be removed here?
>>
>
> I just forgot to remove the "then" on the other comment. There is no
> significance.
>
>
>>
>> >+  auto p_status = symlink_status(p, ec);
>> >+  if (!exists(p_status))
>> >+    {
>> >+      int err = ec.default_error_condition().value();
>> >+      bool not_found = !ec || is_not_found_errno(err);
>> >+      if (not_found)
>> >+      {
>> >+        ec.clear();
>> >+        return 0;
>> >+      }
>> >+      return -1;
>> >+    }
>> >+  if (is_symlink(p_status))
>> >+    {
>> >+      if (remove(p, ec))
>> >+      return 1;
>> >+      return ec ? -1 : 0;
>> >+    }
>> >+#endif
>> >   uintmax_t count = 0;
>> >   recursive_directory_iterator dir(p, directory_options{64|128}, ec);
>> >   switch (ec.value()) // N.B. assumes ec.category() ==
>> std::generic_category()
>> >diff --git a/libstdc++-v3/src/filesystem/ops-common.h
>> b/libstdc++-v3/src/filesystem/ops-common.h
>> >index 4feacfdb932..424bab9b55d 100644
>> >--- a/libstdc++-v3/src/filesystem/ops-common.h
>> >+++ b/libstdc++-v3/src/filesystem/ops-common.h
>> >@@ -120,15 +120,80 @@ namespace __gnu_posix
>> >
>> >   using stat_type = struct ::__stat64;
>> >
>> >-  inline int stat(const wchar_t* path, stat_type* buffer)
>> >-  { return ::_wstat64(path, buffer); }
>> >+#define S_IFLNK 0xC000
>> >+#define       S_ISLNK(m)      (((m) & S_IFMT) == S_IFLNK)
>> >
>> >-  inline int lstat(const wchar_t* path, stat_type* buffer)
>> >+  inline HANDLE __open_for_stat(const wchar_t* path, bool
>> following_symlinks)
>>
>> It seems odd to have these new internal functions in namespace
>> __gnu_posix and then to refer to them as posix::__is_handle_symlink
>> etc.
>>
>> The point of the posix:: namespace is to be a scope for funtions from
>> POSIX (or emulations of those functions for non-POSIX systems).
>>
>> Can we add a different namespace in this header, and put the new
>> helpers in there, and then the Windows versions of posix::stat and
>> posix::lstat would call e.g. __detail::__stat_windows or something
>> like that.
>>
>
> Yeah, I'll do that in v3.
>
>
>>
>> >   {
>> >-    // FIXME: symlinks not currently supported
>> >-    return stat(path, buffer);
>> >+    constexpr auto share_flags
>> >+      = FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE;
>> >+    auto file_flags = FILE_FLAG_BACKUP_SEMANTICS;
>> >+    if (!following_symlinks)
>> >+      file_flags |= FILE_FLAG_OPEN_REPARSE_POINT;
>> >+    HANDLE handle;
>> >+    handle = CreateFileW(path, 0, share_flags, 0, OPEN_EXISTING,
>> file_flags, 0);
>> >+
>> >+    if (handle == INVALID_HANDLE_VALUE)
>> >+      {
>> >+      // CreateFileW does not set errno.
>> >+      std::error_condition generic_error
>> >+        = std::__last_system_error().default_error_condition();
>> >+      errno = generic_error.value();
>> >+      }
>> >+
>> >+    return handle;
>> >   }
>> >
>> >+  // -1 error, 0 not a symlink, 1 a symlink
>> >+  inline int __is_handle_symlink(HANDLE handle)
>> >+  {
>> >+    FILE_ATTRIBUTE_TAG_INFO type_info;
>> >+    if (!GetFileInformationByHandleEx(handle, FileAttributeTagInfo,
>> >+                                    &type_info, sizeof(type_info)))
>> >+      {
>> >+      errno =
>> std::__last_system_error().default_error_condition().value();
>> >+      return -1;
>> >+      }
>> >+    return type_info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT
>> >+         && type_info.ReparseTag == IO_REPARSE_TAG_SYMLINK;
>> >+  }
>> >+
>> >+  inline int __stat_windows(const wchar_t* path, stat_type* buffer,
>> >+                          bool following_symlinks)
>> >+  {
>> >+    HANDLE handle = __open_for_stat(path, following_symlinks);
>> >+    if (handle == INVALID_HANDLE_VALUE)
>> >+      return -1;
>> >+    // Manually check for symlink, because _fstat does not.
>> >+    int is_symlink = __is_handle_symlink(handle);
>> >+    if (is_symlink == -1)
>> >+      {
>> >+      CloseHandle(handle);
>> >+      return -1;
>> >+      }
>> >+    int fd = ::_open_osfhandle((intptr_t)handle, _O_RDONLY);
>> >+    if (fd == -1)
>> >+      {
>> >+      CloseHandle(handle);
>> >+      return -1;
>> >+      }
>> >+    int stat_result = ::_fstat64(fd, buffer);
>> >+    if (is_symlink)
>> >+      {
>> >+      // Clear the previous file type.
>> >+      buffer->st_mode &= ~S_IFMT;
>> >+      buffer->st_mode |= S_IFLNK;
>> >+      }
>> >+    ::_close(fd);
>> >+    return stat_result;
>> >+  }
>> >+
>> >+  inline int stat(const wchar_t* path, stat_type* buffer)
>> >+  { return __stat_windows(path, buffer, true); }
>> >+
>> >+  inline int lstat(const wchar_t* path, stat_type* buffer)
>> >+  { return __stat_windows(path, buffer, false); }
>> >+
>> >   using ::mode_t;
>> >
>> >   inline int chmod(const wchar_t* path, mode_t mode)
>> >diff --git
>> a/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc
>> b/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc
>> >index 6eb7ce28928..6005e423852 100644
>> >--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc
>> >+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/canonical.cc
>> >@@ -118,7 +118,7 @@ test03()
>> >   const fs::path baz = dir/"foo//../bar///";
>> > #endif
>> > #else
>> >-  fs::create_symlink("../bar", foo/"baz");
>> >+  fs::create_directory_symlink("../bar", foo/"baz");
>> >   const fs::path baz = dir/"foo//./baz///";
>> > #endif
>> >
>> >diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
>> b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
>> >index 289bef6160b..50c7b0d1577 100644
>> >--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
>> >+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
>> >@@ -68,13 +68,14 @@ test02()
>> > {
>> > #ifndef NO_SYMLINKS
>> >   const std::error_code bad_ec =
>> make_error_code(std::errc::invalid_argument);
>> >+  __gnu_test::scoped_file tmp_file;
>> >   auto from = __gnu_test::nonexistent_path();
>> >   std::error_code ec;
>> >
>> >   ec = bad_ec;
>> >-  fs::create_symlink(".", from, ec);
>> >+  fs::create_symlink(tmp_file.path, from, ec);
>> >   VERIFY( !ec );
>> >-  VERIFY( fs::exists(from) );
>> >+  VERIFY( fs::exists(symlink_status(from)) );
>> >
>> >   auto to = __gnu_test::nonexistent_path();
>> >   ec = bad_ec;
>> >@@ -97,7 +98,7 @@ test02()
>> >   ec = bad_ec;
>> >   fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
>> >   VERIFY( !ec );
>> >-  VERIFY( fs::exists(to) );
>> >+  VERIFY( fs::exists(symlink_status(to)) );
>> >   VERIFY( is_symlink(to) );
>> >
>> >   ec.clear();
>> >diff --git
>> a/libstdc++-v3/testsuite/27_io/filesystem/operations/weakly_canonical.cc
>> b/libstdc++-v3/testsuite/27_io/filesystem/operations/weakly_canonical.cc
>> >index 6085d5568e6..1c63bf4834d 100644
>> >---
>> a/libstdc++-v3/testsuite/27_io/filesystem/operations/weakly_canonical.cc
>> >+++
>> b/libstdc++-v3/testsuite/27_io/filesystem/operations/weakly_canonical.cc
>> >@@ -40,7 +40,7 @@ test01()
>> >   fs::path p;
>> >
>> > #ifndef NO_SYMLINKS
>> >-  fs::create_symlink("../bar", foo/"bar");
>> >+  fs::create_directory_symlink("../bar", foo/"bar");
>> >
>> >   p = fs::weakly_canonical(dir/"foo//./bar///../biz/.");
>> >   VERIFY( p == dirc/"biz/" );
>> >diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h
>> b/libstdc++-v3/testsuite/util/testsuite_fs.h
>> >index 9cf400d87db..cc0264b2563 100644
>> >--- a/libstdc++-v3/testsuite/util/testsuite_fs.h
>> >+++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
>> >@@ -43,8 +43,10 @@ namespace test_fs = std::experimental::filesystem;
>> > #endif
>> >
>> > #ifndef _GLIBCXX_HAVE_SYMLINK
>> >+#ifndef _GLIBCXX_FILESYSTEM_IS_WINDOWS
>> > #define NO_SYMLINKS
>> > #endif
>> >+#endif
>> >
>> > #if !defined (_GLIBCXX_HAVE_SYS_STATVFS_H) \
>> >   && !defined (_GLIBCXX_FILESYSTEM_IS_WINDOWS)
>> >--
>> >2.50.0
>> >
>> >
>>
>>

Reply via email to