From a7d8ecc6e3d970ab58836acf9713440bbacd6984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20B=C3=A1lint=20Misius?= Date: Thu, 19 Jan 2023 16:51:23 +0100 Subject: [PATCH] Make WriteFile replace rather than overwrite This preserves old file if writing the new one fails for some reason. --- src/common/Platform.cpp | 43 ++++++++++++++++++++++++++++++++++++----- src/common/Platform.h | 4 ++-- src/prefs/Prefs.cpp | 2 +- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/common/Platform.cpp b/src/common/Platform.cpp index 2aa776376..fee96aa29 100644 --- a/src/common/Platform.cpp +++ b/src/common/Platform.cpp @@ -1,5 +1,6 @@ #include "Platform.h" #include "resource.h" +#include "tpt-rand.h" #include #include #include @@ -186,9 +187,14 @@ bool RemoveFile(ByteString filename) #endif } -bool RenameFile(ByteString filename, ByteString newFilename) +bool RenameFile(ByteString filename, ByteString newFilename, bool replace) { #ifdef WIN + if (replace) + { + // TODO: we rely on errno but errors from this are available through GetLastError(); fix + return MoveFileExW(WinWiden(filename).c_str(), WinWiden(newFilename).c_str(), MOVEFILE_REPLACE_EXISTING); + } return _wrename(WinWiden(filename).c_str(), WinWiden(newFilename).c_str()) == 0; #else return rename(filename.c_str(), newFilename.c_str()) == 0; @@ -335,16 +341,43 @@ bool ReadFile(std::vector &fileData, ByteString filename) return true; } -bool WriteFile(std::vector fileData, ByteString filename, bool replaceAtomically) +bool WriteFile(const std::vector &fileData, ByteString filename) { - // TODO: replaceAtomically - std::ofstream f(filename, std::ios::binary); - if (f) f.write(&fileData[0], fileData.size()); + auto replace = FileExists(filename); + auto writeFileName = filename; + if (replace) + { + while (true) + { + writeFileName = ByteString::Build(filename, ".temp.", random_gen() % 100000); + if (!FileExists(writeFileName)) + { + break; + } + } + } + std::ofstream f(writeFileName, std::ios::binary); + if (f) + { + f.write(&fileData[0], fileData.size()); + } if (!f) { std::cerr << "WriteFile: " << filename << ": " << strerror(errno) << std::endl; + if (replace) + { + RemoveFile(writeFileName); + } return false; } + if (replace) + { + if (!RenameFile(writeFileName, filename, true)) + { + RemoveFile(writeFileName); + return false; + } + } return true; } diff --git a/src/common/Platform.h b/src/common/Platform.h index 878c829db..53d290aba 100644 --- a/src/common/Platform.h +++ b/src/common/Platform.h @@ -21,7 +21,7 @@ namespace Platform * @return true on success */ bool RemoveFile(ByteString filename); - bool RenameFile(ByteString filename, ByteString newFilename); + bool RenameFile(ByteString filename, ByteString newFilename, bool replace = false); /** * @return true on success @@ -35,7 +35,7 @@ namespace Platform std::vector DirectorySearch(ByteString directory, ByteString search, std::vector extensions); bool ReadFile(std::vector &fileData, ByteString filename); - bool WriteFile(std::vector fileData, ByteString filename, bool replaceAtomically = false); // TODO: Revisit call sites, remove default. + bool WriteFile(const std::vector &fileData, ByteString filename); ByteString WinNarrow(const std::wstring &source); std::wstring WinWiden(const ByteString &source); diff --git a/src/prefs/Prefs.cpp b/src/prefs/Prefs.cpp index 64eba774c..0da5968f2 100644 --- a/src/prefs/Prefs.cpp +++ b/src/prefs/Prefs.cpp @@ -47,7 +47,7 @@ void Prefs::Write() Json::StreamWriterBuilder wbuilder; wbuilder["indentation"] = "\t"; ByteString data = Json::writeString(wbuilder, root); - if (!Platform::WriteFile(std::vector(data.begin(), data.end()), path, true)) + if (!Platform::WriteFile(std::vector(data.begin(), data.end()), path)) { return; }