public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Björn Schäpers" <gcc@hazardy.de>
To: gcc-patches@gcc.gnu.org, libstdc++@gcc.gnu.org
Cc: "Björn Schäpers" <bjoern@hazardy.de>
Subject: [PATCH] Fix overwriting files with fs::copy_file on windows
Date: Sun, 24 Mar 2024 22:34:01 +0100	[thread overview]
Message-ID: <20240324213401.47870-1-gcc@hazardy.de> (raw)

From: Björn Schäpers <bjoern@hazardy.de>

This fixes i.e. https://github.com/msys2/MSYS2-packages/issues/1937
I don't know if I picked the right way to do it.

When acceptable I think the declaration should be moved into
ops-common.h, since then we could use stat_type and also use that in the
commonly used function.

Manually tested on i686-w64-mingw32.

-- >8 --
libstdc++: Fix overwriting files on windows

The inodes have no meaning on windows, thus all files have an inode of
0. Use a differenz approach to identify equivalent files. As a result
std::filesystem::copy_file did not honor
copy_options::overwrite_existing. Factored the method out of
std::filesystem::equivalent.

libstdc++-v3/Changelog:

	* include/bits/fs_ops.h: Add declaration of
	  __detail::equivalent_win32.
	* src/c++17/fs_ops.cc (__detail::equivalent_win32): Implement it
	(fs::equivalent): Use __detail::equivalent_win32, factored the
	old test out.
	* src/filesystem/ops-common.h (_GLIBCXX_FILESYSTEM_IS_WINDOWS):
	  Use the function.

Signed-off-by: Björn Schäpers <bjoern@hazardy.de>
---
 libstdc++-v3/include/bits/fs_ops.h       |  8 +++
 libstdc++-v3/src/c++17/fs_ops.cc         | 79 +++++++++++++-----------
 libstdc++-v3/src/filesystem/ops-common.h | 10 ++-
 3 files changed, 60 insertions(+), 37 deletions(-)

diff --git a/libstdc++-v3/include/bits/fs_ops.h b/libstdc++-v3/include/bits/fs_ops.h
index 90650c47b46..d10b78a4bdd 100644
--- a/libstdc++-v3/include/bits/fs_ops.h
+++ b/libstdc++-v3/include/bits/fs_ops.h
@@ -40,6 +40,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 namespace filesystem
 {
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+namespace __detail
+{
+  bool
+  equivalent_win32(const wchar_t* p1, const wchar_t* p2, error_code& ec);
+} // namespace __detail
+#endif //_GLIBCXX_FILESYSTEM_IS_WINDOWS
+
   /** @addtogroup filesystem
    *  @{
    */
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 61df19753ef..3cc87d45237 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -67,6 +67,49 @@
 namespace fs = std::filesystem;
 namespace posix = std::filesystem::__gnu_posix;
 
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+bool
+fs::__detail::equivalent_win32(const wchar_t* p1, const wchar_t* p2,
+			       error_code& ec)
+{
+  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);
+  if (!h1 || !h2)
+    {
+      if (!h1 && !h2)
+	ec = __last_system_error();
+      return false;
+    }
+  if (!h1.get_info() || !h2.get_info())
+    {
+      ec = __last_system_error();
+      return false;
+    }
+  return h1.info.dwVolumeSerialNumber == h2.info.dwVolumeSerialNumber
+    && h1.info.nFileIndexHigh == h2.info.nFileIndexHigh
+    && h1.info.nFileIndexLow == h2.info.nFileIndexLow;
+}
+#endif //_GLIBCXX_FILESYSTEM_IS_WINDOWS
+
 fs::path
 fs::absolute(const path& p)
 {
@@ -858,41 +901,7 @@ 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);
-      if (!h1 || !h2)
-	{
-	  if (!h1 && !h2)
-	    ec = __last_system_error();
-	  return false;
-	}
-      if (!h1.get_info() || !h2.get_info())
-	{
-	  ec = __last_system_error();
-	  return false;
-	}
-      return h1.info.dwVolumeSerialNumber == h2.info.dwVolumeSerialNumber
-	&& h1.info.nFileIndexHigh == h2.info.nFileIndexHigh
-	&& h1.info.nFileIndexLow == h2.info.nFileIndexLow;
+      return __detail::equivalent_win32(p1.c_str(), p2.c_str(), ec);
 #else
       return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
 #endif
diff --git a/libstdc++-v3/src/filesystem/ops-common.h b/libstdc++-v3/src/filesystem/ops-common.h
index d917fddbeb1..7e67286bd01 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -489,8 +489,14 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
 	    return false;
 	  }
 
-	if (to_st->st_dev == from_st->st_dev
-	    && to_st->st_ino == from_st->st_ino)
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+	// st_ino is not set, so can't be used to distinguish files
+	std::error_code not_used;
+	if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev ||
+	    fs::__detail::equivalent_win32(from, to, not_used))
+#else
+	if (st1.st_dev != st2.st_dev || st1.st_ino != st2.st_ino)
+#endif
 	  {
 	    ec = std::make_error_code(std::errc::file_exists);
 	    return false;
-- 
2.44.0


             reply	other threads:[~2024-03-24 21:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-24 21:34 Björn Schäpers [this message]
2024-04-25 20:16 ` Björn Schäpers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240324213401.47870-1-gcc@hazardy.de \
    --to=gcc@hazardy.de \
    --cc=bjoern@hazardy.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).