Fixed QFile::copy/rename fail after initial failed attempt

These functions were checking the error state after calling close(),
without first resetting the error state. Turns out close() only resets
the error state if isOpen() returns false.

Also, the fallback for the copy operation opens the file for reading
but wasn't closing it again afterwards. Now fixed.

Added autotests to cover these situations.

Reviewed-by: MariusSO

2 files changed, 62 insertions, 0 deletions

diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp
index d7da800..04750b0 100644
--- a/src/corelib/io/qfile.cpp
+++ b/src/corelib/io/qfile.cpp
@@ -708,6 +708,7 @@ QFile::rename(const QString &newName)
         d->setError(QFile::RenameError, tr("Destination file exists"));
         return false;
     }
+    unsetError();
     close();
     if(error() == QFile::NoError) {
         if (fileEngine()->rename(newName)) {
@@ -849,6 +850,7 @@ QFile::copy(const QString &newName)
         d->setError(QFile::CopyError, tr("Destination file exists"));
         return false;
     }
+    unsetError();
     close();
     if(error() == QFile::NoError) {
         if(fileEngine()->copy(newName)) {
@@ -908,6 +910,7 @@ QFile::copy(const QString &newName)
                         out.setAutoRemove(false);
 #endif
                 }
+                close();
             }
             if(!error) {
                 QFile::setPermissions(newName, permissions());
diff --git a/tests/auto/qfile/tst_qfile.cpp b/tests/auto/qfile/tst_qfile.cpp
index 98e1859..cb8091b 100644
--- a/tests/auto/qfile/tst_qfile.cpp
+++ b/tests/auto/qfile/tst_qfile.cpp
@@ -123,6 +123,7 @@ private slots:
     void permissions();
     void setPermissions();
     void copy();
+    void copyAfterFail();
     void copyRemovesTemporaryFile() const;
     void copyShouldntOverwrite();
     void link();
@@ -216,8 +217,15 @@ void tst_QFile::cleanup()
     // for renameFallback()
     QFile::remove("file-rename-destination.txt");
 
+    // for copyAfterFail()
+    QFile::remove("file-to-be-copied.txt");
+    QFile::remove("existing-file.txt");
+    QFile::remove("copied-file-1.txt");
+    QFile::remove("copied-file-2.txt");
+
     // for renameMultiple()
     QFile::remove("file-to-be-renamed.txt");
+    QFile::remove("existing-file.txt");
     QFile::remove("file-renamed-once.txt");
     QFile::remove("file-renamed-twice.txt");
 }
@@ -886,6 +894,39 @@ void tst_QFile::copy()
     QFile::copy(QDir::currentPath(), QDir::currentPath() + QLatin1String("/test2"));
 }
 
+void tst_QFile::copyAfterFail()
+{
+    QFile file1("file-to-be-copied.txt");
+    QFile file2("existing-file.txt");
+
+    QVERIFY(file1.open(QIODevice::ReadWrite) && "(test-precondition)");
+    QVERIFY(file2.open(QIODevice::ReadWrite) && "(test-precondition)");
+    QVERIFY(!QFile::exists("copied-file-1.txt") && "(test-precondition)");
+    QVERIFY(!QFile::exists("copied-file-2.txt") && "(test-precondition)");
+
+    QVERIFY(!file1.copy("existing-file.txt"));
+    QCOMPARE(file1.error(), QFile::CopyError);
+
+    QVERIFY(file1.copy("copied-file-1.txt"));
+    QVERIFY(!file1.isOpen());
+    QCOMPARE(file1.error(), QFile::NoError);
+
+    QVERIFY(!file1.copy("existing-file.txt"));
+    QCOMPARE(file1.error(), QFile::CopyError);
+
+    QVERIFY(file1.copy("copied-file-2.txt"));
+    QVERIFY(!file1.isOpen());
+    QCOMPARE(file1.error(), QFile::NoError);
+
+    QVERIFY(QFile::exists("copied-file-1.txt"));
+    QVERIFY(QFile::exists("copied-file-2.txt"));
+
+    QVERIFY(QFile::remove("file-to-be-copied.txt") && "(test-cleanup)");
+    QVERIFY(QFile::remove("existing-file.txt") && "(test-cleanup)");
+    QVERIFY(QFile::remove("copied-file-1.txt") && "(test-cleanup)");
+    QVERIFY(QFile::remove("copied-file-2.txt") && "(test-cleanup)");
+}
+
 void tst_QFile::copyRemovesTemporaryFile() const
 {
     const QString newName(QLatin1String("copyRemovesTemporaryFile"));
@@ -2087,24 +2128,42 @@ void tst_QFile::renameMultiple()
 {
     // create the file if it doesn't exist
     QFile file("file-to-be-renamed.txt");
+    QFile file2("existing-file.txt");
     QVERIFY(file.open(QIODevice::ReadWrite) && "(test-precondition)");
+    QVERIFY(file2.open(QIODevice::ReadWrite) && "(test-precondition)");
 
     // any stale files from previous test failures?
     QFile::remove("file-renamed-once.txt");
     QFile::remove("file-renamed-twice.txt");
 
     // begin testing
+    QVERIFY(QFile::exists("existing-file.txt"));
+    QVERIFY(!file.rename("existing-file.txt"));
+    QCOMPARE(file.error(), QFile::RenameError);
+    QCOMPARE(file.fileName(), QString("file-to-be-renamed.txt"));
+
     QVERIFY(file.rename("file-renamed-once.txt"));
+    QVERIFY(!file.isOpen());
     QCOMPARE(file.fileName(), QString("file-renamed-once.txt"));
+
+    QVERIFY(QFile::exists("existing-file.txt"));
+    QVERIFY(!file.rename("existing-file.txt"));
+    QCOMPARE(file.error(), QFile::RenameError);
+    QCOMPARE(file.fileName(), QString("file-renamed-once.txt"));
+
     QVERIFY(file.rename("file-renamed-twice.txt"));
+    QVERIFY(!file.isOpen());
     QCOMPARE(file.fileName(), QString("file-renamed-twice.txt"));
 
+    QVERIFY(QFile::exists("existing-file.txt"));
     QVERIFY(!QFile::exists("file-to-be-renamed.txt"));
     QVERIFY(!QFile::exists("file-renamed-once.txt"));
     QVERIFY(QFile::exists("file-renamed-twice.txt"));
 
     file.remove();
+    file2.remove();
     QVERIFY(!QFile::exists("file-renamed-twice.txt"));
+    QVERIFY(!QFile::exists("existing-file.txt"));
 }
 
 void tst_QFile::appendAndRead()