QFileSystemEngine: make factory functions return unique_ptr<QABFE>

This makes the ownership of the returned pointer clearer. It also
matches reality, some call sites were already storing the pointer in a
unique_ptr.

Also shorten the function name to "createLegacyEngine", you have to read
its docs anyway to figure out what it does.

Drive-by changes: less magic numbers; use sliced(); return nullptr
instead of `0`.

Change-Id: I637759b4160b28b15adf5f6548de336887338dab
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>

src/corelib/doc/snippets/code/src_corelib_io_qabstractfileengine.cpp

@@ -1,17 +1,21 @@
 // Copyright (C) 2016 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR BSD-3-Clause
 
+using namespace Qt::StringLiterals;
+
 //! [0]
 class ZipEngineHandler : public QAbstractFileEngineHandler
 {
 public:
-    QAbstractFileEngine *create(const QString &fileName) const override;
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override;
 };
 
-QAbstractFileEngine *ZipEngineHandler::create(const QString &fileName) const
+std::unique_ptr<QAbstractFileEngine> ZipEngineHandler::create(const QString &fileName) const
 {
     // ZipEngineHandler returns a ZipEngine for all .zip files
-    return fileName.toLower().endsWith(".zip") ? new ZipEngine(fileName) : 0;
+    if (fileName.toLower().endsWith(".zip"_L1))
+        return std::make_unique<ZipEngine>(fileName);
+    return {};
 }
 
 int main(int argc, char **argv)
@@ -27,12 +31,14 @@ int main(int argc, char **argv)
 }
 //! [0]
 
-
 //! [1]
-QAbstractSocketEngine *ZipEngineHandler::create(const QString &fileName) const
+std::unique_ptr<QAbstractFileEngine> ZipEngineHandler::create(const QString &fileName) const
 {
     // ZipEngineHandler returns a ZipEngine for all .zip files
-    return fileName.toLower().endsWith(".zip") ? new ZipEngine(fileName) : 0;
+    if (fileName.toLower().endsWith(".zip"_L1))
+        return std::make_unique<ZipEngine>(fileName);
+    else
+        return {};
 }
 //! [1]
 

src/corelib/io/qabstractfileengine.cpp

@@ -128,14 +128,14 @@ QAbstractFileEngineHandler::~QAbstractFileEngineHandler()
 
    Handles calls to custom file engine handlers.
 */
-QAbstractFileEngine *qt_custom_file_engine_handler_create(const QString &path)
+std::unique_ptr<QAbstractFileEngine> qt_custom_file_engine_handler_create(const QString &path)
 {
     if (qt_file_engine_handlers_in_use.loadRelaxed()) {
         QReadLocker locker(fileEngineHandlerMutex());
 
         // check for registered handlers that can load the file
         for (QAbstractFileEngineHandler *handler : std::as_const(*fileEngineHandlers())) {
-            if (QAbstractFileEngine *engine = handler->create(path))
+            if (auto engine = handler->create(path))
                 return engine;
         }
     }
@@ -144,10 +144,11 @@ QAbstractFileEngine *qt_custom_file_engine_handler_create(const QString &path)
 }
 
 /*!
-    \fn QAbstractFileEngine *QAbstractFileEngineHandler::create(const QString &fileName) const
+    \fn std::unique_ptr<QAbstractFileEngine> QAbstractFileEngineHandler::create(const QString &fileName) const
 
-    Creates a file engine for file \a fileName. Returns 0 if this
+    If this file handler can handle \a fileName, this method creates a file
-    file handler cannot handle \a fileName.
+    engine and returns it wrapped in a std::unique_ptr; otherwise returns
+    nullptr.
 
     Example:
 
@@ -169,16 +170,15 @@ QAbstractFileEngine *qt_custom_file_engine_handler_create(const QString &path)
 
     \sa QAbstractFileEngineHandler
 */
-QAbstractFileEngine *QAbstractFileEngine::create(const QString &fileName)
+std::unique_ptr<QAbstractFileEngine> QAbstractFileEngine::create(const QString &fileName)
 {
     QFileSystemEntry entry(fileName);
     QFileSystemMetaData metaData;
-    QAbstractFileEngine *engine = QFileSystemEngine::resolveEntryAndCreateLegacyEngine(entry, metaData);
+    auto engine = QFileSystemEngine::createLegacyEngine(entry, metaData);
 
 #ifndef QT_NO_FSFILEENGINE
-    if (!engine)
+    if (!engine) // fall back to regular file engine
-        // fall back to regular file engine
+        engine = std::make_unique<QFSFileEngine>(entry.filePath());
-        return new QFSFileEngine(entry.filePath());
 #endif
 
     return engine;

src/corelib/io/qabstractfileengine_p.h

@@ -171,7 +171,7 @@ public:
     virtual bool supportsExtension(Extension extension) const;
 
     // Factory
-    static QAbstractFileEngine *create(const QString &fileName);
+    static std::unique_ptr<QAbstractFileEngine> create(const QString &fileName);
 
 protected:
     void setError(QFile::FileError error, const QString &str);
@@ -192,7 +192,7 @@ class Q_CORE_EXPORT QAbstractFileEngineHandler
 public:
     QAbstractFileEngineHandler();
     virtual ~QAbstractFileEngineHandler();
-    virtual QAbstractFileEngine *create(const QString &fileName) const = 0;
+    virtual std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const = 0;
 };
 
 class Q_CORE_EXPORT QAbstractFileEngineIterator
@@ -242,7 +242,7 @@ public:
     Q_DECLARE_PUBLIC(QAbstractFileEngine)
 };
 
-QAbstractFileEngine *qt_custom_file_engine_handler_create(const QString &path);
+std::unique_ptr<QAbstractFileEngine> qt_custom_file_engine_handler_create(const QString &path);
 
 QT_END_NAMESPACE
 

src/corelib/io/qdir.cpp

@@ -362,8 +362,7 @@ inline void QDirPrivate::clearCache(MetaDataClearing mode)
     fileCache.fileListsInitialized = false;
     fileCache.files.clear();
     fileCache.fileInfos.clear();
-    fileEngine.reset(
+    fileEngine = QFileSystemEngine::createLegacyEngine(dirEntry, fileCache.metaData);
-            QFileSystemEngine::resolveEntryAndCreateLegacyEngine(dirEntry, fileCache.metaData));
 }
 
 /*!

src/corelib/io/qdirlisting.cpp

@@ -139,9 +139,10 @@ void QDirListingPrivate::init(bool resolveEngine = true)
         nameRegExps.emplace_back(QRegularExpression::fromWildcard(filter, cs));
 #endif
 
-    if (resolveEngine)
+    if (resolveEngine) {
-        engine.reset(QFileSystemEngine::resolveEntryAndCreateLegacyEngine(
+        engine = QFileSystemEngine::createLegacyEngine(initialEntryInfo.entry,
-            initialEntryInfo.entry, initialEntryInfo.metaData));
+                                                       initialEntryInfo.metaData);
+    }
 
     pushDirectory(initialEntryInfo);
 }

src/corelib/io/qfile.cpp

@@ -76,7 +76,7 @@ QFilePrivate::openExternalFile(QIODevice::OpenMode flags, FILE *fh, QFile::FileH
 QAbstractFileEngine *QFilePrivate::engine() const
 {
     if (!fileEngine)
-        fileEngine.reset(QAbstractFileEngine::create(fileName));
+        fileEngine = QAbstractFileEngine::create(fileName);
     return fileEngine.get();
 }
 

src/corelib/io/qfileinfo.cpp

@@ -746,10 +746,8 @@ bool QFileInfo::exists(const QString &path)
         return false;
     QFileSystemEntry entry(path);
     QFileSystemMetaData data;
-    std::unique_ptr<QAbstractFileEngine> engine
-        {QFileSystemEngine::resolveEntryAndCreateLegacyEngine(entry, data)};
     // Expensive fallback to non-QFileSystemEngine implementation
-    if (engine)
+    if (auto engine = QFileSystemEngine::createLegacyEngine(entry, data))
         return QFileInfo(new QFileInfoPrivate(entry, data, std::move(engine))).exists();
 
     QFileSystemEngine::fillMetaData(entry, data, QFileSystemMetaData::ExistsAttribute);

src/corelib/io/qfileinfo_p.h

@@ -55,7 +55,7 @@ public:
         : QSharedData(copy),
         fileEntry(copy.fileEntry),
         metaData(copy.metaData),
-        fileEngine(QFileSystemEngine::resolveEntryAndCreateLegacyEngine(fileEntry, metaData)),
+        fileEngine(QFileSystemEngine::createLegacyEngine(fileEntry, metaData)),
         cachedFlags(0),
 #ifndef QT_NO_FSFILEENGINE
         isDefaultConstructed(false),
@@ -66,7 +66,7 @@ public:
     {}
     inline QFileInfoPrivate(const QString &file)
         : fileEntry(file),
-        fileEngine(QFileSystemEngine::resolveEntryAndCreateLegacyEngine(fileEntry, metaData)),
+        fileEngine(QFileSystemEngine::createLegacyEngine(fileEntry, metaData)),
         cachedFlags(0),
 #ifndef QT_NO_FSFILEENGINE
         isDefaultConstructed(file.isEmpty()),
@@ -81,7 +81,7 @@ public:
         : QSharedData(),
         fileEntry(file),
         metaData(data),
-        fileEngine(QFileSystemEngine::resolveEntryAndCreateLegacyEngine(fileEntry, metaData)),
+        fileEngine(QFileSystemEngine::createLegacyEngine(fileEntry, metaData)),
         cachedFlags(0),
         isDefaultConstructed(false),
         cache_enabled(true), fileFlags(0), fileSize(0)

src/corelib/io/qfilesystemengine.cpp

@@ -83,12 +83,11 @@ static inline bool _q_checkEntry(QFileSystemEntry &entry, QFileSystemMetaData &d
     return true;
 }
 
-static inline bool _q_checkEntry(QAbstractFileEngine *&engine, bool resolvingEntry)
+static inline bool _q_checkEntry(std::unique_ptr<QAbstractFileEngine> &engine, bool resolvingEntry)
 {
     if (resolvingEntry) {
         if (!(engine->fileFlags(QAbstractFileEngine::FlagsMask) & QAbstractFileEngine::ExistsFlag)) {
-            delete engine;
+            engine.reset();
-            engine = nullptr;
             return false;
         }
     }
@@ -96,8 +95,9 @@ static inline bool _q_checkEntry(QAbstractFileEngine *&engine, bool resolvingEnt
     return true;
 }
 
-static bool _q_resolveEntryAndCreateLegacyEngine_recursive(QFileSystemEntry &entry, QFileSystemMetaData &data,
+static bool _q_createLegacyEngine_recursive(QFileSystemEntry &entry, QFileSystemMetaData &data,
-        QAbstractFileEngine *&engine, bool resolvingEntry = false)
+                                            std::unique_ptr<QAbstractFileEngine> &engine,
+                                            bool resolvingEntry = false)
 {
     QString const &filePath = entry.filePath();
     if ((engine = qt_custom_file_engine_handler_create(filePath)))
@@ -111,7 +111,7 @@ static bool _q_resolveEntryAndCreateLegacyEngine_recursive(QFileSystemEntry &ent
 
         if (ch == u':') {
             if (prefixSeparator == 0) {
-                engine = new QResourceFileEngine(filePath);
+                engine = std::make_unique<QResourceFileEngine>(filePath);
                 return _q_checkEntry(engine, resolvingEntry);
             }
 
@@ -123,7 +123,7 @@ static bool _q_resolveEntryAndCreateLegacyEngine_recursive(QFileSystemEntry &ent
                 entry = QFileSystemEntry(QDir::cleanPath(
                         paths.at(i) % u'/' % QStringView{filePath}.mid(prefixSeparator + 1)));
                 // Recurse!
-                if (_q_resolveEntryAndCreateLegacyEngine_recursive(entry, data, engine, true))
+                if (_q_createLegacyEngine_recursive(entry, data, engine, true))
                     return true;
             }
 
@@ -153,12 +153,13 @@ static bool _q_resolveEntryAndCreateLegacyEngine_recursive(QFileSystemEntry &ent
     QFileSystemEngine API should be used to query and interact with the file
     system object.
 */
-QAbstractFileEngine *QFileSystemEngine::resolveEntryAndCreateLegacyEngine(
+std::unique_ptr<QAbstractFileEngine>
-        QFileSystemEntry &entry, QFileSystemMetaData &data) {
+QFileSystemEngine::createLegacyEngine(QFileSystemEntry &entry, QFileSystemMetaData &data)
+{
     QFileSystemEntry copy = entry;
-    QAbstractFileEngine *engine = nullptr;
+    std::unique_ptr<QAbstractFileEngine> engine;
 
-    if (_q_resolveEntryAndCreateLegacyEngine_recursive(copy, data, engine))
+    if (_q_createLegacyEngine_recursive(copy, data, engine))
         // Reset entry to resolved copy.
         entry = copy;
     else

src/corelib/io/qfilesystemengine_p.h

@@ -20,6 +20,7 @@
 #include "qfilesystemmetadata_p.h"
 #include <QtCore/private/qsystemerror_p.h>
 
+#include <memory>
 #include <optional>
 
 QT_BEGIN_NAMESPACE
@@ -141,8 +142,9 @@ public:
     static bool setCurrentPath(const QFileSystemEntry &entry);
     static QFileSystemEntry currentPath();
 
-    static QAbstractFileEngine *resolveEntryAndCreateLegacyEngine(QFileSystemEntry &entry,
+    static std::unique_ptr<QAbstractFileEngine>
-                                                                  QFileSystemMetaData &data);
+    createLegacyEngine(QFileSystemEntry &entry, QFileSystemMetaData &data);
+
 private:
     static QString slowCanonicalized(const QString &path);
 #if defined(Q_OS_WIN)

src/corelib/io/qsavefile.cpp

@@ -200,7 +200,7 @@ bool QSaveFile::open(OpenMode mode)
     }
 
     auto openDirectly = [&]() {
-        d->fileEngine.reset(QAbstractFileEngine::create(d->finalFileName));
+        d->fileEngine = QAbstractFileEngine::create(d->finalFileName);
         if (d->fileEngine->open(mode | QIODevice::Unbuffered)) {
             d->useTemporaryFile = false;
             QFileDevice::open(mode);

src/plugins/platforms/android/androidcontentfileengine.cpp

@@ -258,12 +258,14 @@ AndroidContentFileEngine::beginEntryList(const QString &path, QDir::Filters filt
 AndroidContentFileEngineHandler::AndroidContentFileEngineHandler() = default;
 AndroidContentFileEngineHandler::~AndroidContentFileEngineHandler() = default;
 
-QAbstractFileEngine* AndroidContentFileEngineHandler::create(const QString &fileName) const
+std::unique_ptr<QAbstractFileEngine>
+AndroidContentFileEngineHandler::create(const QString &fileName) const
 {
-    if (!fileName.startsWith("content"_L1))
+    if (fileName.startsWith("content"_L1))
-        return nullptr;
+        return std::make_unique<AndroidContentFileEngine>(fileName);
+
+    return {};
 
-    return new AndroidContentFileEngine(fileName);
 }
 
 AndroidContentFileEngineIterator::AndroidContentFileEngineIterator(

src/plugins/platforms/android/androidcontentfileengine.h

@@ -46,7 +46,7 @@ class AndroidContentFileEngineHandler : public QAbstractFileEngineHandler
 public:
     AndroidContentFileEngineHandler();
     ~AndroidContentFileEngineHandler();
-    QAbstractFileEngine *create(const QString &fileName) const override;
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override;
 };
 
 class AndroidContentFileEngineIterator : public QAbstractFileEngineIterator

src/plugins/platforms/android/qandroidassetsfileenginehandler.cpp

@@ -379,13 +379,14 @@ AndroidAssetsFileEngineHandler::AndroidAssetsFileEngineHandler()
     m_assetManager = QtAndroid::assetManager();
 }
 
-QAbstractFileEngine * AndroidAssetsFileEngineHandler::create(const QString &fileName) const
+std::unique_ptr<QAbstractFileEngine>
+AndroidAssetsFileEngineHandler::create(const QString &fileName) const
 {
     if (fileName.isEmpty())
-        return nullptr;
+        return {};
 
     if (!fileName.startsWith(assetsPrefix))
-        return nullptr;
+        return {};
 
     QString path = fileName.mid(prefixSize);
     path.replace("//"_L1, "/"_L1);
@@ -393,7 +394,7 @@ QAbstractFileEngine * AndroidAssetsFileEngineHandler::create(const QString &file
         path.remove(0, 1);
     if (path.endsWith(u'/'))
         path.chop(1);
-    return new AndroidAbstractFileEngine(m_assetManager, path);
+    return std::make_unique<AndroidAbstractFileEngine>(m_assetManager, path);
 }
 
 QT_END_NAMESPACE

src/plugins/platforms/android/qandroidassetsfileenginehandler.h

@@ -17,7 +17,7 @@ class AndroidAssetsFileEngineHandler: public QAbstractFileEngineHandler
 {
 public:
     AndroidAssetsFileEngineHandler();
-    QAbstractFileEngine *create(const QString &fileName) const override;
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override;
 
 private:
     AAssetManager *m_assetManager;

src/plugins/platforms/ios/optional/nsphotolibrarysupport/qiosfileenginefactory.h

@@ -13,18 +13,18 @@ QT_BEGIN_NAMESPACE
 class QIOSFileEngineFactory : public QAbstractFileEngineHandler
 {
 public:
-    QAbstractFileEngine* create(const QString &fileName) const
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const
     {
         Q_CONSTINIT static QLatin1StringView assetsScheme("assets-library:");
 
 #ifndef Q_OS_TVOS
         if (fileName.toLower().startsWith(assetsScheme))
-            return new QIOSFileEngineAssetsLibrary(fileName);
+            return std::make_unique<QIOSFileEngineAssetsLibrary>(fileName);
 #else
         Q_UNUSED(fileName);
 #endif
 
-        return 0;
+        return {};
     }
 };
 

tests/auto/corelib/io/qabstractfileengine/tst_qabstractfileengine.cpp

@@ -17,6 +17,8 @@
 #include <QtCore/QDebug>
 #include "../../../../shared/filesystem.h"
 
+using namespace Qt::StringLiterals;
+
 class tst_QAbstractFileEngine
     : public QObject
 {
@@ -488,17 +490,23 @@ QHash<QString, QSharedPointer<ReferenceFileEngine::File> > ReferenceFileEngine::
 class FileEngineHandler
     : QAbstractFileEngineHandler
 {
-    QAbstractFileEngine *create(const QString &fileName) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override
     {
         if (fileName.endsWith(".tar") || fileName.contains(".tar/"))
-            return new MountingFileEngine(fileName);
+            return std::make_unique<MountingFileEngine>(fileName);
-        if (fileName.startsWith("QFSFileEngine:"))
+
-            return new QFSFileEngine(fileName.mid(14));
+        if (auto l1 = "QFSFileEngine:"_L1; fileName.startsWith(l1))
-        if (fileName.startsWith("reference-file-engine:"))
+            return std::make_unique<QFSFileEngine>(fileName.sliced(l1.size()));
-            return new ReferenceFileEngine(fileName.mid(22));
+
-        if (fileName.startsWith("resource:"))
+        if (auto l1 = "reference-file-engine:"_L1; fileName.startsWith(l1))
-            return QAbstractFileEngine::create(QLatin1String(":/tst_qabstractfileengine/resources/") + fileName.mid(9));
+            return std::make_unique<ReferenceFileEngine>(fileName.sliced(l1.size()));
-        return 0;
+
+        if (auto l1 = "resource:"_L1; fileName.startsWith(l1)) {
+            const auto p = ":/tst_qabstractfileengine/resources/"_L1 + fileName.sliced(l1.size());
+            return QAbstractFileEngine::create(p);
+        }
+
+        return nullptr;
     }
 };
 
@@ -530,9 +538,9 @@ void tst_QAbstractFileEngine::cleanupTestCase()
 
 void tst_QAbstractFileEngine::customHandler()
 {
-    QScopedPointer<QAbstractFileEngine> file;
+    std::unique_ptr<QAbstractFileEngine> file;
     {
-        file.reset(QAbstractFileEngine::create("resource:file.txt"));
+        file = QAbstractFileEngine::create(u"resource:file.txt"_s);
 
         QVERIFY(file);
     }

tests/auto/corelib/io/qdiriterator/tst_qdiriterator.cpp

@@ -444,9 +444,9 @@ public:
 class EngineWithNoIteratorHandler : public QAbstractFileEngineHandler
 {
 public:
-    QAbstractFileEngine *create(const QString &fileName) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override
     {
-        return new EngineWithNoIterator(fileName);
+        return std::make_unique<EngineWithNoIterator>(fileName);
     }
 };
 #endif
@@ -463,11 +463,11 @@ void tst_QDirIterator::engineWithNoIterator()
 class CustomEngineHandler : public QAbstractFileEngineHandler
 {
 public:
-    QAbstractFileEngine *create(const QString &fileName) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override
     {
         // We want to test QFSFileEngine specifically, so force QDirIterator to use it
         // over the default QFileSystemEngine
-        return new QFSFileEngine(fileName);
+        return std::make_unique<QFSFileEngine>(fileName);
     }
 };
 

tests/auto/corelib/io/qdirlisting/tst_qdirlisting.cpp

@@ -428,9 +428,9 @@ public:
 class EngineWithNoIteratorHandler : public QAbstractFileEngineHandler
 {
 public:
-    QAbstractFileEngine *create(const QString &fileName) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override
     {
-        return new EngineWithNoIterator(fileName);
+        return std::make_unique<EngineWithNoIterator>(fileName);
     }
 };
 #endif
@@ -447,11 +447,11 @@ void tst_QDirListing::engineWithNoIterator()
 class CustomEngineHandler : public QAbstractFileEngineHandler
 {
 public:
-    QAbstractFileEngine *create(const QString &fileName) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override
     {
         // We want to test QFSFileEngine specifically, so force QDirListing to use it
         // over the default QFileSystemEngine
-        return new QFSFileEngine(fileName);
+        return std::make_unique<QFSFileEngine>(fileName);
     }
 };
 

tests/auto/corelib/io/qfile/tst_qfile.cpp

@@ -2321,18 +2321,18 @@ private:
 class MyHandler : public QAbstractFileEngineHandler
 {
 public:
-    inline QAbstractFileEngine *create(const QString &) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &) const override
     {
-        return new MyEngine(1);
+        return std::make_unique<MyEngine>(1);
     }
 };
 
 class MyHandler2 : public QAbstractFileEngineHandler
 {
 public:
-    inline QAbstractFileEngine *create(const QString &) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &) const override
     {
-        return new MyEngine(2);
+        return std::make_unique<MyEngine>(2);
     }
 };
 #endif
@@ -2361,7 +2361,7 @@ void tst_QFile::fileEngineHandler()
 class MyRecursiveHandler : public QAbstractFileEngineHandler
 {
 public:
-    inline QAbstractFileEngine *create(const QString &fileName) const override
+    std::unique_ptr<QAbstractFileEngine> create(const QString &fileName) const override
     {
         if (fileName.startsWith(":!")) {
             QDir dir;
@@ -2372,9 +2372,9 @@ public:
             const QString realFile = m_dataDir->filePath(fileName.mid(2));
 #endif
             if (dir.exists(realFile))
-                return new QFSFileEngine(realFile);
+                return std::make_unique<QFSFileEngine>(realFile);
         }
-        return 0;
+        return nullptr;
     }
 
 #ifdef BUILTIN_TESTDATA