From 3c50ad828861bee82e53469deab28b4e286cbeda Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Sat, 10 Feb 2024 03:10:44 +0200 Subject: QFileSystemEngine: make factory functions return unique_ptr 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 Reviewed-by: Assam Boudjelthia --- .../code/src_corelib_io_qabstractfileengine.cpp | 18 +++++++++++------ src/corelib/io/qabstractfileengine.cpp | 20 +++++++++---------- src/corelib/io/qabstractfileengine_p.h | 6 +++--- src/corelib/io/qdir.cpp | 3 +-- src/corelib/io/qdirlisting.cpp | 7 ++++--- src/corelib/io/qfile.cpp | 2 +- src/corelib/io/qfileinfo.cpp | 4 +--- src/corelib/io/qfileinfo_p.h | 6 +++--- src/corelib/io/qfilesystemengine.cpp | 23 +++++++++++----------- src/corelib/io/qfilesystemengine_p.h | 6 ++++-- src/corelib/io/qsavefile.cpp | 2 +- .../platforms/android/androidcontentfileengine.cpp | 10 ++++++---- .../platforms/android/androidcontentfileengine.h | 2 +- .../android/qandroidassetsfileenginehandler.cpp | 9 +++++---- .../android/qandroidassetsfileenginehandler.h | 2 +- .../nsphotolibrarysupport/qiosfileenginefactory.h | 6 +++--- 16 files changed, 68 insertions(+), 58 deletions(-) (limited to 'src') diff --git a/src/corelib/doc/snippets/code/src_corelib_io_qabstractfileengine.cpp b/src/corelib/doc/snippets/code/src_corelib_io_qabstractfileengine.cpp index 557ff765a7..4300fc2da9 100644 --- a/src/corelib/doc/snippets/code/src_corelib_io_qabstractfileengine.cpp +++ b/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 create(const QString &fileName) const override; }; -QAbstractFileEngine *ZipEngineHandler::create(const QString &fileName) const +std::unique_ptr 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(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 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(fileName); + else + return {}; } //! [1] diff --git a/src/corelib/io/qabstractfileengine.cpp b/src/corelib/io/qabstractfileengine.cpp index e7a3d10d01..2db99e5861 100644 --- a/src/corelib/io/qabstractfileengine.cpp +++ b/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 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 QAbstractFileEngineHandler::create(const QString &fileName) const - Creates a file engine for file \a fileName. Returns 0 if this - file handler cannot handle \a fileName. + If this file handler can handle \a fileName, this method creates a file + 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::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) - // fall back to regular file engine - return new QFSFileEngine(entry.filePath()); + if (!engine) // fall back to regular file engine + engine = std::make_unique(entry.filePath()); #endif return engine; diff --git a/src/corelib/io/qabstractfileengine_p.h b/src/corelib/io/qabstractfileengine_p.h index e2524a2d41..3201e014a4 100644 --- a/src/corelib/io/qabstractfileengine_p.h +++ b/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 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 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 qt_custom_file_engine_handler_create(const QString &path); QT_END_NAMESPACE diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp index be0409a01c..9291201d88 100644 --- a/src/corelib/io/qdir.cpp +++ b/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( - QFileSystemEngine::resolveEntryAndCreateLegacyEngine(dirEntry, fileCache.metaData)); + fileEngine = QFileSystemEngine::createLegacyEngine(dirEntry, fileCache.metaData); } /*! diff --git a/src/corelib/io/qdirlisting.cpp b/src/corelib/io/qdirlisting.cpp index bff441ab64..008003e856 100644 --- a/src/corelib/io/qdirlisting.cpp +++ b/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) - engine.reset(QFileSystemEngine::resolveEntryAndCreateLegacyEngine( - initialEntryInfo.entry, initialEntryInfo.metaData)); + if (resolveEngine) { + engine = QFileSystemEngine::createLegacyEngine(initialEntryInfo.entry, + initialEntryInfo.metaData); + } pushDirectory(initialEntryInfo); } diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index 6d1a38160a..6da428f58c 100644 --- a/src/corelib/io/qfile.cpp +++ b/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(); } diff --git a/src/corelib/io/qfileinfo.cpp b/src/corelib/io/qfileinfo.cpp index 8e10467b7b..f607681c82 100644 --- a/src/corelib/io/qfileinfo.cpp +++ b/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 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); diff --git a/src/corelib/io/qfileinfo_p.h b/src/corelib/io/qfileinfo_p.h index 4da7c60792..16ba53259f 100644 --- a/src/corelib/io/qfileinfo_p.h +++ b/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) diff --git a/src/corelib/io/qfilesystemengine.cpp b/src/corelib/io/qfilesystemengine.cpp index bedace4c79..d8b215816c 100644 --- a/src/corelib/io/qfilesystemengine.cpp +++ b/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 &engine, bool resolvingEntry) { if (resolvingEntry) { if (!(engine->fileFlags(QAbstractFileEngine::FlagsMask) & QAbstractFileEngine::ExistsFlag)) { - delete engine; - engine = nullptr; + engine.reset(); 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, - QAbstractFileEngine *&engine, bool resolvingEntry = false) +static bool _q_createLegacyEngine_recursive(QFileSystemEntry &entry, QFileSystemMetaData &data, + std::unique_ptr &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(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( - QFileSystemEntry &entry, QFileSystemMetaData &data) { +std::unique_ptr +QFileSystemEngine::createLegacyEngine(QFileSystemEntry &entry, QFileSystemMetaData &data) +{ QFileSystemEntry copy = entry; - QAbstractFileEngine *engine = nullptr; + std::unique_ptr engine; - if (_q_resolveEntryAndCreateLegacyEngine_recursive(copy, data, engine)) + if (_q_createLegacyEngine_recursive(copy, data, engine)) // Reset entry to resolved copy. entry = copy; else diff --git a/src/corelib/io/qfilesystemengine_p.h b/src/corelib/io/qfilesystemengine_p.h index c1dddb2e1e..78b8337f35 100644 --- a/src/corelib/io/qfilesystemengine_p.h +++ b/src/corelib/io/qfilesystemengine_p.h @@ -20,6 +20,7 @@ #include "qfilesystemmetadata_p.h" #include +#include #include QT_BEGIN_NAMESPACE @@ -141,8 +142,9 @@ public: static bool setCurrentPath(const QFileSystemEntry &entry); static QFileSystemEntry currentPath(); - static QAbstractFileEngine *resolveEntryAndCreateLegacyEngine(QFileSystemEntry &entry, - QFileSystemMetaData &data); + static std::unique_ptr + createLegacyEngine(QFileSystemEntry &entry, QFileSystemMetaData &data); + private: static QString slowCanonicalized(const QString &path); #if defined(Q_OS_WIN) diff --git a/src/corelib/io/qsavefile.cpp b/src/corelib/io/qsavefile.cpp index 3a0fccbe1d..9041775d99 100644 --- a/src/corelib/io/qsavefile.cpp +++ b/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); diff --git a/src/plugins/platforms/android/androidcontentfileengine.cpp b/src/plugins/platforms/android/androidcontentfileengine.cpp index d0a22e35ab..ee57f535d2 100644 --- a/src/plugins/platforms/android/androidcontentfileengine.cpp +++ b/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 +AndroidContentFileEngineHandler::create(const QString &fileName) const { - if (!fileName.startsWith("content"_L1)) - return nullptr; + if (fileName.startsWith("content"_L1)) + return std::make_unique(fileName); + + return {}; - return new AndroidContentFileEngine(fileName); } AndroidContentFileEngineIterator::AndroidContentFileEngineIterator( diff --git a/src/plugins/platforms/android/androidcontentfileengine.h b/src/plugins/platforms/android/androidcontentfileengine.h index ee43185eab..f56d48dbb0 100644 --- a/src/plugins/platforms/android/androidcontentfileengine.h +++ b/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 create(const QString &fileName) const override; }; class AndroidContentFileEngineIterator : public QAbstractFileEngineIterator diff --git a/src/plugins/platforms/android/qandroidassetsfileenginehandler.cpp b/src/plugins/platforms/android/qandroidassetsfileenginehandler.cpp index d85efdd773..4ea6536cef 100644 --- a/src/plugins/platforms/android/qandroidassetsfileenginehandler.cpp +++ b/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 +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(m_assetManager, path); } QT_END_NAMESPACE diff --git a/src/plugins/platforms/android/qandroidassetsfileenginehandler.h b/src/plugins/platforms/android/qandroidassetsfileenginehandler.h index 50c6914c24..8e5f87a51a 100644 --- a/src/plugins/platforms/android/qandroidassetsfileenginehandler.h +++ b/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 create(const QString &fileName) const override; private: AAssetManager *m_assetManager; diff --git a/src/plugins/platforms/ios/optional/nsphotolibrarysupport/qiosfileenginefactory.h b/src/plugins/platforms/ios/optional/nsphotolibrarysupport/qiosfileenginefactory.h index f545a81bf2..136716f792 100644 --- a/src/plugins/platforms/ios/optional/nsphotolibrarysupport/qiosfileenginefactory.h +++ b/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 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(fileName); #else Q_UNUSED(fileName); #endif - return 0; + return {}; } }; -- cgit v1.2.3