From 90f039fa115fb616e4299b07ea393c81a6f1a7a4 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Tue, 9 Jul 2019 18:52:06 +0200
Subject: [PATCH] ENH: support Foam::mv overwrite of existing files on windows
(#1238)
- the behaviour of std::rename with overwriting an existing file is
implementation dependent:
- POSIX: it overwrites.
- Windows: it does not overwrite.
- for Windows need to use the ::MoveFileEx() routine for overwriting.
More investigation is needed for proper handling of very long names.
---
applications/test/copyFile/Test-copyFile.C | 105 +++++++++++----------
src/OSspecific/MSwindows/MSwindows.C | 29 +++++-
src/OSspecific/POSIX/POSIX.C | 8 +-
3 files changed, 85 insertions(+), 57 deletions(-)
diff --git a/applications/test/copyFile/Test-copyFile.C b/applications/test/copyFile/Test-copyFile.C
index 7d6de05d0a..5747287606 100644
--- a/applications/test/copyFile/Test-copyFile.C
+++ b/applications/test/copyFile/Test-copyFile.C
@@ -37,42 +37,68 @@ using namespace Foam;
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
-// Prefix '\\?\' for extended-length path
-inline std::string longName(const std::string& file)
+unsigned maxPath = MAX_PATH;
+
+// Prefix "\\?\" for extended-length path and widen
+// The prefix is only valid with absolute paths
+//
+// In the future, this code will be relocated in MSwindows.C
+
+static inline std::wstring longName(const fileName& file)
{
- std::string longName;
- longName.reserve(4 + file.length());
+ const auto len = file.length();
- longName.append("\\\\?\\");
- longName.append(file);
+ std::wstring out;
+ out.reserve(4 + len);
- return longName;
-}
+ if (len > maxPath)
+ {
+ if (file.isAbsolute())
+ {
+ out.append(L"\\\\?\\");
+ }
+ else
+ {
+ Warning
+ << "Relative fileName exceeds " << maxPath
+ << " characters" << nl
+ << " " << file << nl << nl;
+ }
+ }
-bool win_cp(const fileName& src, const fileName& dst)
-{
- const std::string srcName(longName(src));
- const std::string dstName(longName(dst));
+ // Character-wise copy to get widening
+ for (const auto c : file)
+ {
+ out += c;
+ }
- return ::CopyFile(srcName.c_str(), dstName.c_str(), false);
+ return out;
}
+
bool win_mv(const fileName& src, const fileName& dst)
{
- const std::string srcName(longName(src));
- const std::string dstName(longName(dst));
-
- return ::MoveFile(srcName.c_str(), dstName.c_str());
-}
+ constexpr const int flags
+ (
+ MOVEFILE_COPY_ALLOWED
+ | MOVEFILE_REPLACE_EXISTING
+ | MOVEFILE_WRITE_THROUGH
+ );
+
+ if (src.length() > maxPath || dst.length() > maxPath)
+ {
+ const std::wstring srcName(longName(src));
+ const std::wstring dstName(longName(dst));
-#else
+ Info<< "Windows mv (wide)" << nl;
+ return ::MoveFileExW(srcName.c_str(), dstName.c_str(), flags);
+ }
-bool win_cp(const fileName& src, const fileName& dst)
-{
- Info<< "No Windows cp, using Foam::cp" << nl;
- return Foam::cp(src, dst);
+ Info<< "Windows mv (ansi)" << nl;
+ return ::MoveFileExA(src.c_str(), dst.c_str(), flags);
}
+#else
bool win_mv(const fileName& src, const fileName& dst)
{
@@ -95,8 +121,7 @@ int main(int argc, char *argv[])
#ifdef _WIN32
argList::addBoolOption("win1", "Foam cp, Windows mv");
- argList::addBoolOption("win2", "Windows cp, Foam mv");
- argList::addBoolOption("win3", "Windows cp, Windows mv");
+ argList::addOption("maxPath", "length", "Test with shorter MAX_PATH");
#endif
argList::addArgument("srcFile");
@@ -104,6 +129,10 @@ int main(int argc, char *argv[])
#include "setRootCase.H"
+ #ifdef _WIN32
+ args.readIfPresent("maxPath", maxPath);
+ #endif
+
const fileName srcFile(fileName::validate(args[1]));
const fileName dstFile(fileName::validate(args[2]));
const fileName tmpFile(dstFile + Foam::name(pid()));
@@ -111,11 +140,13 @@ int main(int argc, char *argv[])
Info<< "src : " << srcFile << nl
<< "tmp : " << tmpFile << nl
<< "dst : " << dstFile << nl
+ #ifdef _WIN32
+ << "test with max-path: " << maxPath << nl
+ #endif
<< nl;
bool cpOk = false, mvOk = false;
-
if (args.found("win1"))
{
const auto usage = argList::optionUsage.cfind("win1");
@@ -127,28 +158,6 @@ int main(int argc, char *argv[])
cpOk = Foam::cp(srcFile, tmpFile);
mvOk = win_mv(tmpFile, dstFile);
}
- else if (args.found("win2"))
- {
- const auto usage = argList::optionUsage.cfind("win2");
- if (usage.good())
- {
- Info<< " " << (*usage).c_str() << nl;
- }
-
- cpOk = win_cp(srcFile, tmpFile);
- mvOk = Foam::mv(tmpFile, dstFile);
- }
- else if (args.found("win3"))
- {
- const auto usage = argList::optionUsage.cfind("win3");
- if (usage.good())
- {
- Info<< " " << (*usage).c_str() << nl;
- }
-
- cpOk = win_cp(srcFile, tmpFile);
- mvOk = win_mv(tmpFile, dstFile);
- }
else
{
Info<< " Foam cp, Foam mv" << nl;
diff --git a/src/OSspecific/MSwindows/MSwindows.C b/src/OSspecific/MSwindows/MSwindows.C
index 9ea78be680..105302d478 100644
--- a/src/OSspecific/MSwindows/MSwindows.C
+++ b/src/OSspecific/MSwindows/MSwindows.C
@@ -24,6 +24,9 @@ License
You should have received a copy of the GNU General Public License
along with OpenFOAM. If not, see <http://www.gnu.org/licenses/>.
+Description
+ MS-Windows versions of the functions declared in OSspecific.H
+
\*---------------------------------------------------------------------------*/
#include "OSspecific.H"
@@ -77,6 +80,22 @@ namespace Foam
static bool const abortHandlerInstalled = installAbortHandler();
+
+ // Move file, overwriting existing
+ static bool renameFile(const fileName& src, const fileName& dst)
+ {
+ constexpr const int flags
+ (
+ MOVEFILE_COPY_ALLOWED
+ | MOVEFILE_REPLACE_EXISTING
+ | MOVEFILE_WRITE_THROUGH
+ );
+
+ // TODO: handle extra-long paths with ::MoveFileExW
+
+ return ::MoveFileExA(src.c_str(), dst.c_str(), flags);
+ }
+
} // End namespace Foam
@@ -928,10 +947,10 @@ bool Foam::mv(const fileName& src, const fileName& dst, const bool followLink)
{
const fileName dstName(dst/src.name());
- return 0 == std::rename(src.c_str(), dstName.c_str());
+ return renameFile(src, dstName);
}
- return 0 == std::rename(src.c_str(), dst.c_str());
+ return renameFile(src, dst);
}
@@ -945,10 +964,10 @@ bool Foam::mvBak(const fileName& src, const std::string& ext)
if (exists(src, false))
{
- const int maxIndex = 99;
+ constexpr const int maxIndex = 99;
char index[3];
- for (int n = 0; n <= maxIndex; n++)
+ for (int n = 0; n <= maxIndex; ++n)
{
fileName dstName(src + "." + ext);
if (n)
@@ -961,7 +980,7 @@ bool Foam::mvBak(const fileName& src, const std::string& ext)
// possible index where we have no choice
if (!exists(dstName, false) || n == maxIndex)
{
- return (0 == std::rename(src.c_str(), dstName.c_str()));
+ return renameFile(src, dstName);
}
}
}
diff --git a/src/OSspecific/POSIX/POSIX.C b/src/OSspecific/POSIX/POSIX.C
index e7ad0f4565..f5bfae1a1f 100644
--- a/src/OSspecific/POSIX/POSIX.C
+++ b/src/OSspecific/POSIX/POSIX.C
@@ -1213,10 +1213,10 @@ bool Foam::mv(const fileName& src, const fileName& dst, const bool followLink)
{
const fileName dstName(dst/src.name());
- return (0 == ::rename(src.c_str(), dstName.c_str()));
+ return (0 == std::rename(src.c_str(), dstName.c_str()));
}
- return (0 == ::rename(src.c_str(), dst.c_str()));
+ return (0 == std::rename(src.c_str(), dst.c_str()));
}
@@ -1241,7 +1241,7 @@ bool Foam::mvBak(const fileName& src, const std::string& ext)
if (exists(src, false))
{
- const int maxIndex = 99;
+ constexpr const int maxIndex = 99;
char index[3];
for (int n = 0; n <= maxIndex; ++n)
@@ -1257,7 +1257,7 @@ bool Foam::mvBak(const fileName& src, const std::string& ext)
// possible index where we have no choice
if (!exists(dstName, false) || n == maxIndex)
{
- return (0 == ::rename(src.c_str(), dstName.c_str()));
+ return (0 == std::rename(src.c_str(), dstName.c_str()));
}
}
}
--
GitLab