Fix FileSaver and add some autotests to make sure it works

Change-Id: I707edb703068868014b4434b7508bfa41617383c
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

3 files changed, 40 insertions, 15 deletions

diff --git a/src/lib/corelib/tools/filesaver.cpp b/src/lib/corelib/tools/filesaver.cpp
index db795b012..0fd652657 100644
--- a/src/lib/corelib/tools/filesaver.cpp
+++ b/src/lib/corelib/tools/filesaver.cpp
@@ -40,11 +40,9 @@
 #include "filesaver.h"
 #include "stlutils.h"
 
-#include <QtCore/qfile.h>
-#include <QtCore/qsavefile.h>
-
 #include <tools/iosutils.h>
 
+#include <cerrno>
 #include <fstream>
 
 namespace qbs {
@@ -71,34 +69,50 @@ bool FileSaver::open()
             m_oldFileContents.clear();
     }
 
-    m_newFileContents.clear();
-    m_memoryDevice = std::make_shared<std::ostringstream>(m_newFileContents);
+    m_memoryDevice = std::make_shared<std::stringstream>();
     return true;
 }
 
 bool FileSaver::commit()
 {
-    if (!m_overwriteIfUnchanged && m_oldFileContents == m_newFileContents)
+    if (!device())
+        return false;
+
+    device()->flush();
+    if (!device()->good())
+        return false;
+
+    const std::string newFileContents = m_memoryDevice->str();
+    if (!m_overwriteIfUnchanged && m_oldFileContents == newFileContents)
         return true; // no need to write unchanged data
 
-    const std::string tempFilePath = std::tmpnam(nullptr);
+    const std::string tempFilePath = m_filePath + "~";
     std::ofstream tempFile(utf8_to_native_path(tempFilePath));
     if (!tempFile.is_open())
         return false;
 
-    tempFile.write(m_newFileContents.data(), m_newFileContents.size());
+    tempFile.write(newFileContents.data(), newFileContents.size());
+    tempFile.close();
     if (!tempFile.good())
         return false;
 
-    return Internal::rename(tempFilePath, m_filePath) == 0;
+    if (Internal::rename(tempFilePath, m_filePath) != 0) {
+        if (errno != EEXIST)
+            return false;
+        if (Internal::unlink(m_filePath) != 0)
+            return false;
+        return Internal::rename(tempFilePath, m_filePath) == 0;
+    }
+
+    return true;
 }
 
-size_t FileSaver::write(const std::vector<char> &data)
+bool FileSaver::write(const std::vector<char> &data)
 {
     return fwrite(data, device());
 }
 
-size_t FileSaver::write(const std::string &data)
+bool FileSaver::write(const std::string &data)
 {
     return fwrite(data, device());
 }
diff --git a/src/lib/corelib/tools/filesaver.h b/src/lib/corelib/tools/filesaver.h
index bee04405f..275ad0f01 100644
--- a/src/lib/corelib/tools/filesaver.h
+++ b/src/lib/corelib/tools/filesaver.h
@@ -60,13 +60,12 @@ public:
     std::ostream *device();
     bool open();
     bool commit();
-    size_t write(const std::vector<char> &data);
-    size_t write(const std::string &data);
+    bool write(const std::vector<char> &data);
+    bool write(const std::string &data);
 
 private:
-    std::string m_newFileContents;
     std::string m_oldFileContents;
-    std::shared_ptr<std::ostringstream> m_memoryDevice;
+    std::shared_ptr<std::stringstream> m_memoryDevice;
     const std::string m_filePath;
     const bool m_overwriteIfUnchanged;
 };
diff --git a/src/lib/corelib/tools/iosutils.h b/src/lib/corelib/tools/iosutils.h
index 3fc7b0356..9374b24b1 100644
--- a/src/lib/corelib/tools/iosutils.h
+++ b/src/lib/corelib/tools/iosutils.h
@@ -41,15 +41,19 @@
 #define QBS_IOSUTILS_H
 
 #include <cstdio>
+#include <cstring>
 #include <ostream>
 
 #if defined(_WIN32) && defined(_MSC_VER)
 #include <codecvt>
 #include <locale>
 #define QBS_RENAME_IMPL ::_wrename
+#define QBS_UNLINK_IMPL ::_wunlink
 typedef std::wstring qbs_filesystem_path_string_type;
 #else
+#include <unistd.h>
 #define QBS_RENAME_IMPL ::rename
+#define QBS_UNLINK_IMPL ::unlink
 typedef std::string qbs_filesystem_path_string_type;
 #endif
 
@@ -58,6 +62,8 @@ namespace Internal {
 
 static inline bool fwrite(const char *values, size_t nitems, std::ostream *stream)
 {
+    if (!stream)
+        return false;
     stream->write(values, nitems);
     return stream->good();
 }
@@ -90,6 +96,12 @@ static inline int rename(const std::string &oldName, const std::string &newName)
     return QBS_RENAME_IMPL(wOldName.c_str(), wNewName.c_str());
 }
 
+static inline int unlink(const std::string &name)
+{
+    const auto wName = utf8_to_native_path(name);
+    return QBS_UNLINK_IMPL(wName.c_str());
+}
+
 } // namespace Internal
 } // namespace qbs