From 61e0419f3387b349ce65668d8b2c51fc8df4c954 Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Wed, 27 May 2015 12:43:51 -0400
Subject: [PATCH] SystemTools: Teach RemoveFile to tolerate missing file
Some use cases may have a race condition such that the file to be
removed disappears before we remove it. Detect when removal fails due
to the file already missing and tolerate it without failing.
On Windows this requires using DeleteFileW instead of _wunlink because
the latter does not seem to always update errno. Try to delete before
checking permissions because getting permissions will fail if the file
is missing.
Change-Id: If1922a15d742daca6d252c594284800d60cc1fce
---
SystemTools.cxx | 44 ++++++++++++++++++++++++++++++--------------
testSystemTools.cxx | 18 ++++++++++++++++++
2 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/SystemTools.cxx b/SystemTools.cxx
index 6c4a7a6..c834e34 100644
--- a/SystemTools.cxx
+++ b/SystemTools.cxx
@@ -2674,27 +2674,43 @@ kwsys_stl::string SystemTools::GetLastSystemError()
bool SystemTools::RemoveFile(const kwsys_stl::string& source)
{
#ifdef _WIN32
+ kwsys_stl::wstring const& ws =
+ SystemTools::ConvertToWindowsExtendedPath(source);
+ if (DeleteFileW(ws.c_str()))
+ {
+ return true;
+ }
+ DWORD err = GetLastError();
+ if (err == ERROR_FILE_NOT_FOUND ||
+ err == ERROR_PATH_NOT_FOUND)
+ {
+ return true;
+ }
+ if (err != ERROR_ACCESS_DENIED)
+ {
+ return false;
+ }
+ /* The file may be read-only. Try adding write permission. */
mode_t mode;
- if ( !SystemTools::GetPermissions(source, mode) )
+ if (!SystemTools::GetPermissions(source, mode) ||
+ !SystemTools::SetPermissions(source, S_IWRITE))
{
+ SetLastError(err);
return false;
}
- /* Win32 unlink is stupid --- it fails if the file is read-only */
- SystemTools::SetPermissions(source, S_IWRITE);
-#endif
-#ifdef _WIN32
- bool res =
- _wunlink(SystemTools::ConvertToWindowsExtendedPath(source).c_str()) == 0;
-#else
- bool res = unlink(source.c_str()) != 0 ? false : true;
-#endif
-#ifdef _WIN32
- if ( !res )
+ if (DeleteFileW(ws.c_str()) ||
+ GetLastError() == ERROR_FILE_NOT_FOUND ||
+ GetLastError() == ERROR_PATH_NOT_FOUND)
{
- SystemTools::SetPermissions(source, mode);
+ return true;
}
+ /* Try to restore the original permissions. */
+ SystemTools::SetPermissions(source, mode);
+ SetLastError(err);
+ return false;
+#else
+ return unlink(source.c_str()) == 0 || errno == ENOENT;
#endif
- return res;
}
bool SystemTools::RemoveADirectory(const kwsys_stl::string& source)
diff --git a/testSystemTools.cxx b/testSystemTools.cxx
index 42b6249..15d8eab 100644
--- a/testSystemTools.cxx
+++ b/testSystemTools.cxx
@@ -156,6 +156,24 @@ static bool CheckFileOperations()
res = false;
}
+ kwsys_stl::string const testFileMissing(testNewDir + "/testMissingFile.txt");
+ if (!kwsys::SystemTools::RemoveFile(testFileMissing))
+ {
+ std::string const& msg = kwsys::SystemTools::GetLastSystemError();
+ kwsys_ios::cerr <<
+ "RemoveFile(\"" << testFileMissing << "\") failed: " << msg << "\n";
+ res = false;
+ }
+
+ kwsys_stl::string const testFileMissingDir(testNewDir + "/missing/file.txt");
+ if (!kwsys::SystemTools::RemoveFile(testFileMissingDir))
+ {
+ std::string const& msg = kwsys::SystemTools::GetLastSystemError();
+ kwsys_ios::cerr <<
+ "RemoveFile(\"" << testFileMissingDir << "\") failed: " << msg << "\n";
+ res = false;
+ }
+
kwsys::SystemTools::Touch(testNewFile.c_str(), true);
if (!kwsys::SystemTools::RemoveADirectory(testNewDir))
{
--
GitLab