From 0dafe5cb8c8135d365c8cb0d365eae5ce66d268e Mon Sep 17 00:00:00 2001
From: Ivan Donchevskii <ivan.donchevskii@qt.io>
Date: Fri, 15 Dec 2017 09:11:01 +0100
Subject: [PATCH] Utils: improve savefile error messages

Change-Id: I505c33e1e56a67a3682de2e2cbfe2ab967d04242
Reviewed-by: David Schulz <david.schulz@qt.io>
---
 src/libs/utils/savefile.cpp | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/libs/utils/savefile.cpp b/src/libs/utils/savefile.cpp
index 3e00b36beee..b14178ed47c 100644
--- a/src/libs/utils/savefile.cpp
+++ b/src/libs/utils/savefile.cpp
@@ -112,25 +112,35 @@ bool SaveFile::commit()
 
     QString finalFileName
             = FileUtils::resolveSymlinks(FileName::fromString(m_finalFileName)).toString();
-    QString bakname = finalFileName + QLatin1Char('~');
+    const QString backupName = finalFileName + '~';
 
+    // Back up current file.
+    // If it's opened by another application, the lock follows the move.
     if (QFile::exists(finalFileName)) {
-        QFile::remove(bakname); // Kill old backup
-        // Try to back up current file
-        if (!QFile::rename(finalFileName, bakname)) {
+        // Kill old backup. Might be useful if creator crashed before removing backup.
+        QFile::remove(backupName);
+        QFile finalFile(finalFileName);
+        if (!finalFile.rename(backupName)) {
             remove();
-            setErrorString(tr("File might be locked."));
+            setErrorString(finalFile.errorString());
             return false;
         }
     }
-    if (!rename(finalFileName)) { // Replace current file
-        QFile::rename(bakname, finalFileName); // Rollback to current file
-        remove();
-        return false;
-    }
-    QFile::remove(bakname);
 
-    return true;
+    bool result = true;
+    if (!rename(finalFileName)) {
+        // The case when someone else was able to create finalFileName after we've renamed it.
+        // Higher level call may try to save this file again but here we do nothing and
+        // return false while keeping the error string from last rename call.
+        const QString &renameError = errorString();
+        remove();
+        setErrorString(renameError);
+        result = false;
+    }
+
+    QFile::remove(backupName);
+
+    return result;
 }
 
 void SaveFile::initializeUmask()