* [PATCH] Fix overwriting files with fs::copy_file on windows
@ 2024-03-24 21:34 Björn Schäpers
2024-04-25 20:16 ` Björn Schäpers
2024-05-17 14:34 ` Jonathan Wakely
0 siblings, 2 replies; 4+ messages in thread
From: Björn Schäpers @ 2024-03-24 21:34 UTC (permalink / raw)
To: gcc-patches, libstdc++; +Cc: Björn Schäpers
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix overwriting files with fs::copy_file on windows
2024-03-24 21:34 [PATCH] Fix overwriting files with fs::copy_file on windows Björn Schäpers
@ 2024-04-25 20:16 ` Björn Schäpers
2024-05-16 18:49 ` One of your IPs tried to hack me
2024-05-17 14:34 ` Jonathan Wakely
1 sibling, 1 reply; 4+ messages in thread
From: Björn Schäpers @ 2024-04-25 20:16 UTC (permalink / raw)
To: gcc-patches, libstdc++; +Cc: Björn Schäpers
Am 24.03.2024 um 22:34 schrieb Björn Schäpers:
> 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;
A friendly ping.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix overwriting files with fs::copy_file on windows
2024-04-25 20:16 ` Björn Schäpers
@ 2024-05-16 18:49 ` One of your IPs tried to hack me
0 siblings, 0 replies; 4+ messages in thread
From: One of your IPs tried to hack me @ 2024-05-16 18:49 UTC (permalink / raw)
To: gcc-patches, libstdc++, Jonathan Wakely
Am 25.04.2024 um 22:16 schrieb Björn Schäpers:
> Am 24.03.2024 um 22:34 schrieb Björn Schäpers:
>> 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;
>
> A friendly ping.
Ping.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix overwriting files with fs::copy_file on windows
2024-03-24 21:34 [PATCH] Fix overwriting files with fs::copy_file on windows Björn Schäpers
2024-04-25 20:16 ` Björn Schäpers
@ 2024-05-17 14:34 ` Jonathan Wakely
1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2024-05-17 14:34 UTC (permalink / raw)
To: Björn Schäpers; +Cc: gcc-patches, libstdc++, Björn Schäpers
On Sun, 24 Mar 2024 at 21:34, Björn Schäpers <gcc@hazardy.de> wrote:
>
> 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);
I don't think we want this declared in the public header, it should be
internal to the library.
Can it just be declared in ops-common.h instead?
> +} // 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);
This creates a new filesystem::path, just to call c_str() on it
immediately. The auto_handle ctor should be changed to just take const
wchar_t* so we don't need to allocate and parse new path objects.
> + 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-17 14:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-24 21:34 [PATCH] Fix overwriting files with fs::copy_file on windows Björn Schäpers
2024-04-25 20:16 ` Björn Schäpers
2024-05-16 18:49 ` One of your IPs tried to hack me
2024-05-17 14:34 ` Jonathan Wakely
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).