On 30/01/24 17:00 -0500, Lennox Ho wrote:
std::filesystem::hard_link_count() always returns 1 on
mingw-w64ucrt-11.0.1-r3 on Windows 10 19045
hard_link_count() queries _wstat64() on MinGW-w64
The MSFT documentation claims _wstat64() will always return 1 *non*-NTFS volumes
https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2013/14h5k7ff(v=vs.120)
My tests suggest that is not always true -
hard_link_count()/_wstat64() still returns 1 on NTFS.
GetFileInformationByHandle does return the correct result of 2.
Please see the PR for a minimal repro.
This patch changes the Windows implementation to always call
GetFileInformationByHandle.
Ran the libstdc++-v3/testsuite on x86_64-pc-linux-gnu and x86_64-pc-cygwin.
Performed manual testing on x86_64-w64-mingw32.
This is my first GCC contribution attempt. Please call out any
omissions or mistakes. Thanks.
Thanks for the patch, sorry it took so long to review. This is a great
first patch, and I'm going to push it to trunk with minor changes (see
attached).
I have one comment about the new test file, see below.
Apart from that, I've only made small whitespace/indentation changes.
I would note that gmail seems to have mangled the patch, adding line
breaks and changing whitespace. I couldn't apply the patch and had to
make the changes to the code by hand.
Attaching patches as a .txt file works OK in gmail, it doesn't mangle
them if you do that. Even better is to use git-send-email if you have
that configured.
Finally, patches for libstdc++ should be CC'd to the libstdc++ mailing
list as well as the gcc-patches list. Otherwise, they might get missed
by the libstdc++ maintainers (I don't subscribe to the gcc-patches
list).
Thanks again for fixing this.
PR target/113663
libstdc++-v3/ChangeLog:
* src/c++17/fs_ops.cc
(fs::equivalent): Moved helper class auto_handle
to anonymous namespace as auto_win_file_handle.
(fs::hard_link_count): Changed Windows implementation
to use information provided by GetFileInformationByHandle which is
more reliable.
* testsuite/27_io/filesystem/operations/hard_link_count.cc: New test.
Signed-off-by: Lennox Shou Hao Ho <lennox...@gmail.com>
---
libstdc++-v3/src/c++17/fs_ops.cc | 59 ++++++++++++-------
.../filesystem/operations/hard_link_count.cc | 54 +++++++++++++++++
2 files changed, 91 insertions(+), 22 deletions(-)
create mode 100644
libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 61df19753ef..eec3d86590f 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -822,6 +822,31 @@ fs::equivalent(const path& p1, const path& p2)
return result;
}
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+namespace
+{
+ struct auto_win_file_handle {
+ explicit auto_win_file_handle(const fs::path& p_)
+ : handle(CreateFileW(p_.c_str(), 0,
+ FILE_SHARE_DELETE | FILE_SHARE_READ |
FILE_SHARE_WRITE,
+ 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0))
+ { }
+
+ ~auto_win_file_handle()
+ { if (*this) CloseHandle(handle); }
+
+ explicit operator bool() const
+ { return handle != INVALID_HANDLE_VALUE; }
+
+ bool get_info()
+ { return GetFileInformationByHandle(handle, &info); }
+
+ HANDLE handle;
+ BY_HANDLE_FILE_INFORMATION info;
+ };
+}
+#endif
+
bool
fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
{
@@ -858,27 +883,8 @@ fs::equivalent(const path& p1, const path& p2, error_code&
ec) noexcept
if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev)
return false;
- struct auto_handle {
- explicit auto_handle(const path& p_)
- : handle(CreateFileW(p_.c_str(), 0,
- FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
- 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0))
- { }
-
- ~auto_handle()
- { if (*this) CloseHandle(handle); }
-
- explicit operator bool() const
- { return handle != INVALID_HANDLE_VALUE; }
-
- bool get_info()
- { return GetFileInformationByHandle(handle, &info); }
-
- HANDLE handle;
- BY_HANDLE_FILE_INFORMATION info;
- };
- auto_handle h1(p1);
- auto_handle h2(p2);
+ auto_win_file_handle h1(p1);
+ auto_win_file_handle h2(p2);
if (!h1 || !h2)
{
if (!h1 && !h2)
@@ -982,7 +988,16 @@ fs::hard_link_count(const path& p)
std::uintmax_t
fs::hard_link_count(const path& p, error_code& ec) noexcept
{
-#ifdef _GLIBCXX_HAVE_SYS_STAT_H
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+ auto_win_file_handle h(p);
+ if (h && h.get_info())
+ {
+ return static_cast<uintmax_t>(h.info.nNumberOfLinks);
+ }
+
+ ec = __last_system_error();
+ return static_cast<uintmax_t>(-1);
+#elif defined(_GLIBCXX_HAVE_SYS_STAT_H)
return do_stat(p, ec, std::mem_fn(&stat_type::st_nlink),
static_cast<uintmax_t>(-1));
#else
diff --git
a/libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc
b/libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc
new file mode 100644
index 00000000000..5acba96aa82
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc
@@ -0,0 +1,54 @@
+// Copyright (C) 2019-2024 Free Software Foundation, Inc.
This is a new test, so the copyright dates are wrong, and since you
don't have a copyright assignment on file it isn't copyright FSF
either.
There's no need for copyright info on this test anyway, it's not doing
anything inventive or original, just testing the API in obvious ways.
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++17 } }
+// { dg-require-filesystem-ts "" }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::filesystem;
+
+void test01()
+{
+ // PR libstdc++/113663
+
+ fs::path p1 = __gnu_test::nonexistent_path();
+ VERIFY( !fs::exists(p1) );
+
+ __gnu_test::scoped_file f1(p1);
+ VERIFY( fs::exists(p1) );
+
+ VERIFY( fs::hard_link_count(p1) == 1 );
+
+ fs::path p2 = __gnu_test::nonexistent_path();
+ VERIFY( !fs::exists(p2) );
+
+ fs::create_hard_link(p1, p2);
+ __gnu_test::scoped_file f2(p2, __gnu_test::scoped_file::adopt_file);
+ VERIFY( fs::exists(p2) );
+
+ VERIFY( fs::hard_link_count(p1) == 2 );
+ VERIFY( fs::hard_link_count(p2) == 2 );
+}
+
+int
+main()
+{
+ test01();
+}
--
2.34.1
commit 8bf1e1c2c92ea31d771d0ac559ac9691f32a40b4
Author: Lennox Shou Hao Ho <lennox...@gmail.com>
Date: Mon Jul 29 21:09:27 2024
libstdc++: Fix fs::hard_link_count behaviour on MinGW [PR113663]
std::filesystem::hard_link_count() always returns 1 on
mingw-w64ucrt-11.0.1-r3 on Windows 10 19045
hard_link_count() queries _wstat64() on MinGW-w64
The MSFT documentation claims _wstat64() will always return 1 *non*-NTFS volumes
https://learn.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2013/14h5k7ff(v=vs.120)
My tests suggest that is not always true -
hard_link_count()/_wstat64() still returns 1 on NTFS.
GetFileInformationByHandle does return the correct result of 2.
Please see the PR for a minimal repro.
This patch changes the Windows implementation to always call
GetFileInformationByHandle.
PR libstdc++/113663
libstdc++-v3/ChangeLog:
* src/c++17/fs_ops.cc (fs::equivalent): Moved helper class
auto_handle to anonymous namespace as auto_win_file_handle.
(fs::hard_link_count): Changed Windows implementation to use
information provided by GetFileInformationByHandle which is more
reliable.
* testsuite/27_io/filesystem/operations/hard_link_count.cc: New
test.
Signed-off-by: "Lennox" Shou Hao Ho <lennox...@gmail.com>
Reviewed-by: Jonathan Wakely <jwak...@redhat.com>
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 07bc2a0fa88..81227c49dfd 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -822,6 +822,34 @@ fs::equivalent(const path& p1, const path& p2)
return result;
}
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+namespace
+{
+ // An RAII type that opens a handle for an existing file.
+ struct auto_win_file_handle
+ {
+ explicit
+ auto_win_file_handle(const fs::path& p_)
+ : handle(CreateFileW(p_.c_str(), 0,
+ FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
+ 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0))
+ { }
+
+ ~auto_win_file_handle()
+ { if (*this) CloseHandle(handle); }
+
+ explicit operator bool() const
+ { return handle != INVALID_HANDLE_VALUE; }
+
+ bool get_info()
+ { return GetFileInformationByHandle(handle, &info); }
+
+ HANDLE handle;
+ BY_HANDLE_FILE_INFORMATION info;
+ };
+}
+#endif
+
bool
fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
{
@@ -858,27 +886,8 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev)
return false;
- struct auto_handle {
- explicit auto_handle(const path& p_)
- : handle(CreateFileW(p_.c_str(), 0,
- FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
- 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0))
- { }
-
- ~auto_handle()
- { if (*this) CloseHandle(handle); }
-
- explicit operator bool() const
- { return handle != INVALID_HANDLE_VALUE; }
-
- bool get_info()
- { return GetFileInformationByHandle(handle, &info); }
-
- HANDLE handle;
- BY_HANDLE_FILE_INFORMATION info;
- };
- auto_handle h1(p1);
- auto_handle h2(p2);
+ auto_win_file_handle h1(p1);
+ auto_win_file_handle h2(p2);
if (!h1 || !h2)
{
if (!h1 && !h2)
@@ -982,7 +991,13 @@ fs::hard_link_count(const path& p)
std::uintmax_t
fs::hard_link_count(const path& p, error_code& ec) noexcept
{
-#ifdef _GLIBCXX_HAVE_SYS_STAT_H
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+ auto_win_file_handle h(p);
+ if (h && h.get_info())
+ return static_cast<uintmax_t>(h.info.nNumberOfLinks);
+ ec = __last_system_error();
+ return static_cast<uintmax_t>(-1);
+#elif defined _GLIBCXX_HAVE_SYS_STAT_H
return do_stat(p, ec, std::mem_fn(&stat_type::st_nlink),
static_cast<uintmax_t>(-1));
#else
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc
new file mode 100644
index 00000000000..8b2fb4f190e
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/hard_link_count.cc
@@ -0,0 +1,37 @@
+// { dg-do run { target c++17 } }
+// { dg-require-filesystem-ts "" }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::filesystem;
+
+void test01()
+{
+ // PR libstdc++/113663
+
+ fs::path p1 = __gnu_test::nonexistent_path();
+ VERIFY( !fs::exists(p1) );
+
+ __gnu_test::scoped_file f1(p1);
+ VERIFY( fs::exists(p1) );
+
+ VERIFY( fs::hard_link_count(p1) == 1 );
+
+ fs::path p2 = __gnu_test::nonexistent_path();
+ VERIFY( !fs::exists(p2) );
+
+ fs::create_hard_link(p1, p2);
+ __gnu_test::scoped_file f2(p2, __gnu_test::scoped_file::adopt_file);
+ VERIFY( fs::exists(p2) );
+
+ VERIFY( fs::hard_link_count(p1) == 2 );
+ VERIFY( fs::hard_link_count(p2) == 2 );
+}
+
+int
+main()
+{
+ test01();
+}
