QFile: hold engine by unique_ptr

Unfortunately, we can't, yet, change QAbstractFileEngine::create() to
return a unique_ptr. But we should do it in Qt 6.

Change-Id: If18ff766bce73ecd4143274ac9f9a5a7b9d5912c
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/corelib/io/qfile.cpp

@@ -55,6 +55,8 @@
 # include "qcoreapplication.h"
 #endif
 
+#include <private/qmemory_p.h>
+
 #ifdef QT_NO_QOBJECT
 #define tr(X) QString::fromLatin1(X)
 #endif
@@ -85,10 +87,9 @@ QFilePrivate::openExternalFile(int flags, int fd, QFile::FileHandleFlags handleF
     Q_UNUSED(fd);
     return false;
 #else
-    delete fileEngine;
-    fileEngine = nullptr;
-    QFSFileEngine *fe = new QFSFileEngine;
-    fileEngine = fe;
+    auto fs = qt_make_unique<QFSFileEngine>();
+    auto fe = fs.get();
+    fileEngine = std::move(fs);
     return fe->open(QIODevice::OpenMode(flags), fd, handleFlags);
 #endif
 }
@@ -101,10 +102,9 @@ QFilePrivate::openExternalFile(int flags, FILE *fh, QFile::FileHandleFlags handl
     Q_UNUSED(fh);
     return false;
 #else
-    delete fileEngine;
-    fileEngine = nullptr;
-    QFSFileEngine *fe = new QFSFileEngine;
-    fileEngine = fe;
+    auto fs = qt_make_unique<QFSFileEngine>();
+    auto fe = fs.get();
+    fileEngine = std::move(fs);
     return fe->open(QIODevice::OpenMode(flags), fh, handleFlags);
 #endif
 }
@@ -112,8 +112,8 @@ QFilePrivate::openExternalFile(int flags, FILE *fh, QFile::FileHandleFlags handl
 QAbstractFileEngine *QFilePrivate::engine() const
 {
     if (!fileEngine)
-        fileEngine = QAbstractFileEngine::create(fileName);
-    return fileEngine;
+        fileEngine.reset(QAbstractFileEngine::create(fileName));
+    return fileEngine.get();
 }
 
 //************* QFile
@@ -334,10 +334,7 @@ QFile::setFileName(const QString &name)
         file_already_open(*this, "setFileName");
         close();
     }
-    if(d->fileEngine) { //get a new file engine later
-        delete d->fileEngine;
-        d->fileEngine = nullptr;
-    }
+    d->fileEngine.reset(); //get a new file engine later
     d->fileName = name;
 }
 

src/corelib/io/qfiledevice.cpp

@@ -42,6 +42,8 @@
 #include "qfiledevice_p.h"
 #include "qfsfileengine_p.h"
 
+#include <private/qmemory_p.h>
+
 #ifdef QT_NO_QOBJECT
 #define tr(X) QString::fromLatin1(X)
 #endif
@@ -53,24 +55,20 @@ QT_BEGIN_NAMESPACE
 #endif
 
 QFileDevicePrivate::QFileDevicePrivate()
-    : fileEngine(nullptr),
-      cachedSize(0),
+    : cachedSize(0),
       error(QFile::NoError), lastWasWrite(false)
 {
     writeBufferChunkSize = QFILE_WRITEBUFFER_SIZE;
 }
 
 QFileDevicePrivate::~QFileDevicePrivate()
-{
-    delete fileEngine;
-    fileEngine = nullptr;
-}
+    = default;
 
 QAbstractFileEngine * QFileDevicePrivate::engine() const
 {
     if (!fileEngine)
-        fileEngine = new QFSFileEngine;
-    return fileEngine;
+        fileEngine = qt_make_unique<QFSFileEngine>();
+    return fileEngine.get();
 }
 
 void QFileDevicePrivate::setError(QFileDevice::FileError err)

src/corelib/io/qfiledevice_p.h

@@ -53,6 +53,8 @@
 
 #include "private/qiodevice_p.h"
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 
 class QAbstractFileEngine;
@@ -75,7 +77,7 @@ protected:
     void setError(QFileDevice::FileError err, const QString &errorString);
     void setError(QFileDevice::FileError err, int errNum);
 
-    mutable QAbstractFileEngine *fileEngine;
+    mutable std::unique_ptr<QAbstractFileEngine> fileEngine;
     mutable qint64 cachedSize;
 
     QFileDevice::FileHandleFlags handleFlags;

src/corelib/io/qsavefile.cpp

@@ -150,8 +150,7 @@ QSaveFile::~QSaveFile()
     QFileDevice::close();
     if (d->fileEngine) {
         d->fileEngine->remove();
-        delete d->fileEngine;
-        d->fileEngine = nullptr;
+        d->fileEngine.reset();
     }
 }
 
@@ -235,7 +234,7 @@ bool QSaveFile::open(OpenMode mode)
     }
 
     auto openDirectly = [&]() {
-        d->fileEngine = QAbstractFileEngine::create(d->finalFileName);
+        d->fileEngine.reset(QAbstractFileEngine::create(d->finalFileName));
         if (d->fileEngine->open(mode | QIODevice::Unbuffered)) {
             d->useTemporaryFile = false;
             QFileDevice::open(mode);
@@ -252,8 +251,7 @@ bool QSaveFile::open(OpenMode mode)
             if (openDirectly())
                 return true;
             d->setError(d->fileEngine->error(), d->fileEngine->errorString());
-            delete d->fileEngine;
-            d->fileEngine = nullptr;
+            d->fileEngine.reset();
         } else {
             QString msg =
                     QSaveFile::tr("QSaveFile cannot open '%1' without direct write fallback "
@@ -265,18 +263,17 @@ bool QSaveFile::open(OpenMode mode)
     }
 #endif
 
-    d->fileEngine = new QTemporaryFileEngine(&d->finalFileName, QTemporaryFileEngine::Win32NonShared);
+    d->fileEngine.reset(new QTemporaryFileEngine(&d->finalFileName, QTemporaryFileEngine::Win32NonShared));
     // if the target file exists, we'll copy its permissions below,
     // but until then, let's ensure the temporary file is not accessible
     // to a third party
     int perm = (existingFile.exists() ? 0600 : 0666);
-    static_cast<QTemporaryFileEngine *>(d->fileEngine)->initialize(d->finalFileName, perm);
+    static_cast<QTemporaryFileEngine *>(d->fileEngine.get())->initialize(d->finalFileName, perm);
     // Same as in QFile: QIODevice provides the buffering, so there's no need to request it from the file engine.
     if (!d->fileEngine->open(mode | QIODevice::Unbuffered)) {
         QFileDevice::FileError err = d->fileEngine->error();
 #ifdef Q_OS_UNIX
         if (d->directWriteFallback && err == QFileDevice::OpenError && errno == EACCES) {
-            delete d->fileEngine;
             if (openDirectly())
                 return true;
             err = d->fileEngine->error();
@@ -285,8 +282,7 @@ bool QSaveFile::open(OpenMode mode)
         if (err == QFileDevice::UnspecifiedError)
             err = QFileDevice::OpenError;
         d->setError(err, d->fileEngine->errorString());
-        delete d->fileEngine;
-        d->fileEngine = nullptr;
+        d->fileEngine.reset();
         return false;
     }
 
@@ -332,30 +328,26 @@ bool QSaveFile::commit()
     }
     QFileDevice::close(); // calls flush()
 
+    const auto fe = std::move(d->fileEngine);
+
     // Sync to disk if possible. Ignore errors (e.g. not supported).
-    d->fileEngine->syncToDisk();
+    fe->syncToDisk();
 
     if (d->useTemporaryFile) {
         if (d->writeError != QFileDevice::NoError) {
-            d->fileEngine->remove();
+            fe->remove();
             d->writeError = QFileDevice::NoError;
-            delete d->fileEngine;
-            d->fileEngine = nullptr;
             return false;
         }
         // atomically replace old file with new file
         // Can't use QFile::rename for that, must use the file engine directly
-        Q_ASSERT(d->fileEngine);
-        if (!d->fileEngine->renameOverwrite(d->finalFileName)) {
-            d->setError(d->fileEngine->error(), d->fileEngine->errorString());
-            d->fileEngine->remove();
-            delete d->fileEngine;
-            d->fileEngine = nullptr;
+        Q_ASSERT(fe);
+        if (!fe->renameOverwrite(d->finalFileName)) {
+            d->setError(fe->error(), fe->errorString());
+            fe->remove();
             return false;
         }
     }
-    delete d->fileEngine;
-    d->fileEngine = nullptr;
     return true;
 }
 

src/corelib/io/qtemporaryfile.cpp

@@ -544,10 +544,10 @@ QTemporaryFilePrivate::~QTemporaryFilePrivate()
 QAbstractFileEngine *QTemporaryFilePrivate::engine() const
 {
     if (!fileEngine) {
-        fileEngine = new QTemporaryFileEngine(&templateName);
+        fileEngine.reset(new QTemporaryFileEngine(&templateName));
         resetFileEngine();
     }
-    return fileEngine;
+    return fileEngine.get();
 }
 
 void QTemporaryFilePrivate::resetFileEngine() const
@@ -555,7 +555,7 @@ void QTemporaryFilePrivate::resetFileEngine() const
     if (!fileEngine)
         return;
 
-    QTemporaryFileEngine *tef = static_cast<QTemporaryFileEngine *>(fileEngine);
+    QTemporaryFileEngine *tef = static_cast<QTemporaryFileEngine *>(fileEngine.get());
     if (fileName.isEmpty())
         tef->initialize(templateName, 0600);
     else
@@ -568,7 +568,7 @@ void QTemporaryFilePrivate::materializeUnnamedFile()
     if (!fileName.isEmpty() || !fileEngine)
         return;
 
-    auto *tef = static_cast<QTemporaryFileEngine *>(fileEngine);
+    auto *tef = static_cast<QTemporaryFileEngine *>(fileEngine.get());
     fileName = tef->fileName(QAbstractFileEngine::DefaultName);
 #endif
 }
@@ -792,7 +792,7 @@ void QTemporaryFile::setAutoRemove(bool b)
 QString QTemporaryFile::fileName() const
 {
     Q_D(const QTemporaryFile);
-    auto tef = static_cast<QTemporaryFileEngine *>(d->fileEngine);
+    auto tef = static_cast<QTemporaryFileEngine *>(d->fileEngine.get());
     if (tef && tef->isReallyOpen())
         const_cast<QTemporaryFilePrivate *>(d)->materializeUnnamedFile();
 
@@ -841,7 +841,7 @@ void QTemporaryFile::setFileTemplate(const QString &name)
 bool QTemporaryFile::rename(const QString &newName)
 {
     Q_D(QTemporaryFile);
-    auto tef = static_cast<QTemporaryFileEngine *>(d->fileEngine);
+    auto tef = static_cast<QTemporaryFileEngine *>(d->fileEngine.get());
     if (!tef || !tef->isReallyOpen() || !tef->filePathWasTemplate)
         return QFile::rename(newName);
 
@@ -947,7 +947,7 @@ QTemporaryFile *QTemporaryFile::createNativeFile(QFile &file)
 bool QTemporaryFile::open(OpenMode flags)
 {
     Q_D(QTemporaryFile);
-    auto tef = static_cast<QTemporaryFileEngine *>(d->fileEngine);
+    auto tef = static_cast<QTemporaryFileEngine *>(d->fileEngine.get());
     if (tef && tef->isReallyOpen()) {
         setOpenMode(flags);
         return true;
@@ -961,7 +961,7 @@ bool QTemporaryFile::open(OpenMode flags)
     d->resetFileEngine();
 
     if (QFile::open(flags)) {
-        tef = static_cast<QTemporaryFileEngine *>(d->fileEngine);
+        tef = static_cast<QTemporaryFileEngine *>(d->fileEngine.get());
         if (tef->isUnnamedFile())
             d->fileName.clear();
         else