[PATCH] libstdc++: Fix fs::hard_link_count behaviour on MinGW [PR113663]
Jonathan Wakely
jwakely@redhat.com
Mon Jul 29 20:26:47 GMT 2024
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 <lennoxhoe@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 5026 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20240729/57d4f4f6/attachment-0001.bin>
More information about the Libstdc++
mailing list