https://gcc.gnu.org/g:35c98149a5c7af9159fa7615d8d827b3aaa3cc97
commit r14-10640-g35c98149a5c7af9159fa7615d8d827b3aaa3cc97
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Tue Jul 30 10:55:55 2024 +0100
libstdc++: Fix overwriting files with fs::copy_file on Windows
There are no inode numbers on Windows filesystems, so stat_type::st_ino
is always zero and the check for equivalent files in do_copy_file was
incorrectly identifying distinct files as equivalent. This caused
copy_file to incorrectly report errors when trying to overwrite existing
files.
The fs::equivalent function already does the right thing on Windows, so
factor that logic out into a new function that can be reused by
fs::copy_file.
The tests for fs::copy_file were quite inadequate, so this also adds
checks for that function's error conditions.
libstdc++-v3/ChangeLog:
* src/c++17/fs_ops.cc (auto_win_file_handle): Change constructor
parameter from const path& to const wchar_t*.
(fs::equiv_files): New function.
(fs::equivalent): Use equiv_files.
* src/filesystem/ops-common.h (fs::equiv_files): Declare.
(do_copy_file): Use equiv_files.
* src/filesystem/ops.cc (fs::equiv_files): Define.
(fs::copy, fs::equivalent): Use equiv_files.
* testsuite/27_io/filesystem/operations/copy.cc: Test
overwriting directory contents recursively.
* testsuite/27_io/filesystem/operations/copy_file.cc: Test
overwriting existing files.
(cherry picked from commit 017e3f89b081e4828a588a3bd27b5feacea042b7)
Diff:
---
libstdc++-v3/src/c++17/fs_ops.cc | 71 +++++++-----
libstdc++-v3/src/filesystem/ops-common.h | 12 +-
libstdc++-v3/src/filesystem/ops.cc | 18 ++-
.../testsuite/27_io/filesystem/operations/copy.cc | 9 ++
.../27_io/filesystem/operations/copy_file.cc | 122 +++++++++++++++++++++
5 files changed, 199 insertions(+), 33 deletions(-)
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 81227c49dfd..9606afa9f1f 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -350,7 +350,7 @@ fs::copy(const path& from, const path& to, copy_options
options,
f = make_file_status(from_st);
if (exists(t) && !is_other(t) && !is_other(f)
- && to_st.st_dev == from_st.st_dev && to_st.st_ino == from_st.st_ino)
+ && fs::equiv_files(from.c_str(), from_st, to.c_str(), to_st, ec))
{
ec = std::make_error_code(std::errc::file_exists);
return;
@@ -829,8 +829,8 @@ namespace
struct auto_win_file_handle
{
explicit
- auto_win_file_handle(const fs::path& p_)
- : handle(CreateFileW(p_.c_str(), 0,
+ auto_win_file_handle(const wchar_t* p)
+ : handle(CreateFileW(p, 0,
FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0))
{ }
@@ -850,6 +850,44 @@ namespace
}
#endif
+#ifdef _GLIBCXX_HAVE_SYS_STAT_H
+#ifdef NEED_DO_COPY_FILE // Only define this once, not in cow-fs_ops.o too
+bool
+fs::equiv_files([[maybe_unused]] const char_type* p1, const stat_type& st1,
+ [[maybe_unused]] const char_type* p2, const stat_type& st2,
+ [[maybe_unused]] error_code& ec)
+{
+#if ! _GLIBCXX_FILESYSTEM_IS_WINDOWS
+ // For POSIX the device ID and inode number uniquely identify a file.
+ return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
+#else
+ // For Windows st_ino is not set, so can't be used to distinguish files.
+ // We can compare modes and device IDs as a cheap initial check:
+ if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev)
+ return false;
+
+ // Need to use GetFileInformationByHandle to get more info about the files.
+ auto_win_file_handle h1(p1);
+ auto_win_file_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
+}
+#endif // NEED_DO_COPY_FILE
+#endif // _GLIBCXX_HAVE_SYS_STAT_H
+
bool
fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
{
@@ -881,30 +919,7 @@ fs::equivalent(const path& p1, const path& p2, error_code&
ec) noexcept
ec.clear();
if (is_other(s1) || is_other(s2))
return false;
-#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
- // st_ino is not set, so can't be used to distinguish files
- if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev)
- return false;
-
- auto_win_file_handle h1(p1);
- auto_win_file_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;
-#else
- return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
-#endif
+ return fs::equiv_files(p1.c_str(), st1, p2.c_str(), st2, ec);
}
else if (!exists(s1) || !exists(s2))
ec = std::make_error_code(std::errc::no_such_file_or_directory);
@@ -992,7 +1007,7 @@ std::uintmax_t
fs::hard_link_count(const path& p, error_code& ec) noexcept
{
#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
- auto_win_file_handle h(p);
+ auto_win_file_handle h(p.c_str());
if (h && h.get_info())
return static_cast<uintmax_t>(h.info.nNumberOfLinks);
ec = __last_system_error();
diff --git a/libstdc++-v3/src/filesystem/ops-common.h
b/libstdc++-v3/src/filesystem/ops-common.h
index d917fddbeb1..1be8d754d6f 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -303,7 +303,6 @@ namespace __gnu_posix
{
bool skip, update, overwrite;
};
-
#endif // _GLIBCXX_HAVE_SYS_STAT_H
} // namespace filesystem
@@ -327,6 +326,12 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
std::error_code&);
+ // Test whether two files are the same file.
+ bool
+ equiv_files(const char_type*, const stat_type&,
+ const char_type*, const stat_type&,
+ error_code&);
+
inline file_type
make_file_type(const stat_type& st) noexcept
{
@@ -400,6 +405,7 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
return true;
}
#endif
+
#if defined _GLIBCXX_USE_SENDFILE && ! defined _GLIBCXX_FILESYSTEM_IS_WINDOWS
bool
copy_file_sendfile(int fd_in, int fd_out, size_t length) noexcept
@@ -428,6 +434,7 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
return true;
}
#endif
+
bool
do_copy_file(const char_type* from, const char_type* to,
std::filesystem::copy_options_existing_file options,
@@ -489,8 +496,7 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
return false;
}
- if (to_st->st_dev == from_st->st_dev
- && to_st->st_ino == from_st->st_ino)
+ if (equiv_files(from, *from_st, to, *to_st, ec))
{
ec = std::make_error_code(std::errc::file_exists);
return false;
diff --git a/libstdc++-v3/src/filesystem/ops.cc
b/libstdc++-v3/src/filesystem/ops.cc
index 4d23a804da0..c430f58c47a 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -257,6 +257,20 @@ namespace
} // namespace
+#ifdef _GLIBCXX_HAVE_SYS_STAT_H
+#ifdef NEED_DO_COPY_FILE // Only define this once, not in cow-ops.o too
+bool
+fs::equiv_files([[maybe_unused]] const char_type* p1, const stat_type& st1,
+ [[maybe_unused]] const char_type* p2, const stat_type& st2,
+ [[maybe_unused]] error_code& ec)
+{
+ // For POSIX the device ID and inode number uniquely identify a file.
+ // This doesn't work on Windows (see equiv_files in src/c++17/fs_ops.cc).
+ return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
+}
+#endif
+#endif
+
void
fs::copy(const path& from, const path& to, copy_options options,
error_code& ec) noexcept
@@ -293,7 +307,7 @@ fs::copy(const path& from, const path& to, copy_options
options,
f = make_file_status(from_st);
if (exists(t) && !is_other(t) && !is_other(f)
- && to_st.st_dev == from_st.st_dev && to_st.st_ino == from_st.st_ino)
+ && fs::equiv_files(from.c_str(), from_st, to.c_str(), to_st, ec))
{
ec = std::make_error_code(std::errc::file_exists);
return;
@@ -763,7 +777,7 @@ fs::equivalent(const path& p1, const path& p2, error_code&
ec) noexcept
ec.clear();
if (is_other(s1) || is_other(s2))
return false;
- return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
+ return fs::equiv_files(p1.c_str(), st1, p2.c_str(), st2, ec);
}
else if (!exists(s1) || !exists(s2))
ec = std::make_error_code(std::errc::no_such_file_or_directory);
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
index 777a9b8402e..893f32297ae 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
@@ -216,6 +216,15 @@ test_pr99290()
}
#endif
+ // https://github.com/msys2/MSYS2-packages/issues/1937
+ auto copy_opts
+ = fs::copy_options::overwrite_existing | fs::copy_options::recursive;
+ copy(source, dest, copy_opts, ec);
+ VERIFY( !ec );
+
+ auto ch = std::ifstream{dest/"file"}.get();
+ VERIFY( ch == 'a' );
+
remove_all(dir);
}
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
index e3a1219fa6e..af643151044 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
@@ -76,8 +76,130 @@ test01()
remove(to);
}
+void
+test_directory()
+{
+ std::error_code ec;
+
+ auto dir = __gnu_test::nonexistent_path();
+ auto file = __gnu_test::nonexistent_path();
+
+ // error condition: is_regular_file(from) is false
+ create_directory(dir);
+ bool b = copy_file(dir, file, ec);
+ VERIFY( !b );
+ VERIFY( ec == std::errc::invalid_argument );
+ VERIFY( !exists(file) );
+ VERIFY( is_directory(dir) );
+
+ ec = std::make_error_code(std::errc::address_in_use);
+ // error condition: exists(to) is true and is_regular_file(to) is false
+ std::ofstream{file} << "regular file";
+ b = copy_file(file, dir, ec);
+ VERIFY( !b );
+ VERIFY( ec == std::errc::invalid_argument );
+ VERIFY( exists(file) );
+ VERIFY( is_directory(dir) );
+
+ remove(dir);
+ remove(file);
+}
+
+void
+test_existing()
+{
+ using std::filesystem::copy_options;
+ std::error_code ec;
+
+ auto from = __gnu_test::nonexistent_path();
+ auto to = from;
+
+ // error condition: exists(to) is true and equivalent(from, to) is true
+ std::ofstream{from} << "source";
+ bool b = copy_file(from, to, ec);
+ VERIFY( !b );
+ VERIFY( ec == std::errc::file_exists );
+
+ to = __gnu_test::nonexistent_path();
+ std::ofstream{to} << "overwrite me";
+
+ // error condition: exists(to) is true and options == copy_options::none
+ b = copy_file(from, to, ec);
+ VERIFY( !b );
+ VERIFY( ec == std::errc::file_exists );
+ VERIFY( file_size(to) != file_size(from) );
+
+#if __cpp_exceptions
+ try
+ {
+ (void) copy_file(from, to);
+ VERIFY(false);
+ }
+ catch (const std::filesystem::filesystem_error& e)
+ {
+ std::string_view msg = e.what();
+ VERIFY( msg.find(from.string()) != msg.npos );
+ VERIFY( msg.find(to.string()) != msg.npos );
+ VERIFY( e.code() == std::errc::file_exists );
+ }
+ VERIFY( file_size(to) == 12 );
+#endif
+
+ b = copy_file(from, to, copy_options::skip_existing, ec);
+ VERIFY( !b );
+ VERIFY( !ec );
+ VERIFY( file_size(to) != file_size(from) );
+
+ b = copy_file(from, to, copy_options::skip_existing); // doesn't throw
+ VERIFY( !b );
+ VERIFY( file_size(to) != file_size(from) );
+
+ b = copy_file(from, to, copy_options::overwrite_existing, ec);
+ VERIFY( b );
+ VERIFY( !ec );
+ VERIFY( file_size(to) == file_size(from) );
+
+ std::ofstream{to} << "overwrite me again";
+ b = copy_file(from, to, copy_options::overwrite_existing); // doesn't throw
+ VERIFY( b );
+ VERIFY( file_size(to) == file_size(from) );
+
+ ec = std::make_error_code(std::errc::address_in_use);
+ auto time = last_write_time(from);
+ std::ofstream{to} << "touched";
+ last_write_time(to, time + std::chrono::seconds(60));
+ b = copy_file(from, to, copy_options::update_existing, ec);
+ VERIFY( !b );
+ VERIFY( !ec );
+ VERIFY( file_size(to) != file_size(from) );
+
+ b = copy_file(from, to, copy_options::update_existing); // doesn't throw
+ VERIFY( !b );
+ VERIFY( file_size(to) != file_size(from) );
+
+ ec = std::make_error_code(std::errc::address_in_use);
+ time = last_write_time(to);
+ std::ofstream{from} << "touched more recently";
+ last_write_time(from, time + std::chrono::seconds(60));
+ b = copy_file(from, to, copy_options::update_existing, ec);
+ VERIFY( b );
+ VERIFY( !ec );
+ VERIFY( file_size(to) == file_size(from) );
+
+ time = last_write_time(to);
+ std::ofstream{from} << "touched even more recently";
+ last_write_time(from, time + std::chrono::seconds(60));
+ b = copy_file(from, to, copy_options::update_existing); // doesn't throw
+ VERIFY( b );
+ VERIFY( file_size(to) == file_size(from) );
+
+ remove(from);
+ remove(to);
+}
+
int
main()
{
test01();
+ test_existing();
}
