From 6bc7309135ea6fc6c5d881e988b560e09825059d Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 23 Jul 2019 08:21:28 +0300 Subject: QResource: fix nullptr-check gone tautological Amends 136c5b9338f71775eb42528cfc7c23b2b4e5dff9. Before that change, each of the three members was a separate Q_GLOBAL_STATIC, so checking resourceList() for nullptr was the correct thing to do to find out whether the static was already destroyed. After the change, the resourceList() function will never return nullptr. Either resourceGlobalData.isDestroyed(), in which case dereferencing it asserts, or it isn't, in which case resourceList() returns a valid pointer. An explicit isDestroyed() check was added to the unregister function, but the register one was also checking resourceList() for nullptr, and this was left unprotected. Add the check and remove the now-tautological checks for nullptr resourceList(). Change-Id: I41fe66939ce858a77802b8af04c1de6e4fafe048 Reviewed-by: Edward Welbourne --- src/corelib/io/qresource.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qresource.cpp b/src/corelib/io/qresource.cpp index 564a3e5f51..6c458da77a 100644 --- a/src/corelib/io/qresource.cpp +++ b/src/corelib/io/qresource.cpp @@ -852,8 +852,10 @@ bool QResourceRoot::mappingRootSubdir(const QString &path, QString *match) const Q_CORE_EXPORT bool qRegisterResourceData(int version, const unsigned char *tree, const unsigned char *name, const unsigned char *data) { + if (resourceGlobalData.isDestroyed()) + return false; QMutexLocker lock(resourceMutex()); - if ((version == 0x01 || version == 0x2) && resourceList()) { + if (version == 0x01 || version == 0x2) { bool found = false; QResourceRoot res(version, tree, name, data); for(int i = 0; i < resourceList()->size(); ++i) { @@ -879,7 +881,7 @@ Q_CORE_EXPORT bool qUnregisterResourceData(int version, const unsigned char *tre return false; QMutexLocker lock(resourceMutex()); - if ((version == 0x01 || version == 0x02) && resourceList()) { + if (version == 0x01 || version == 0x02) { QResourceRoot res(version, tree, name, data); for(int i = 0; i < resourceList()->size(); ) { if(*resourceList()->at(i) == res) { -- cgit v1.2.3 From 56b1cb0b040887c7f7a39561c1dc22c82af9ae7b Mon Sep 17 00:00:00 2001 From: Oliver Wolff Date: Tue, 23 Jul 2019 11:10:59 +0200 Subject: Move processOpenModeFlags out of QFSFileEngine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same logic is needed for QWinRTFileEngine. To be able to reuse the code, it was moved out of the class. Task-number: QTBUG-77095 Change-Id: If52b2fc8a0f3056d32fc693775565a1c3803b7d4 Reviewed-by: André de la Rocha Reviewed-by: Edward Welbourne --- src/corelib/io/qfsfileengine.cpp | 45 ++++++++++++++++++++++++++-------------- src/corelib/io/qfsfileengine_p.h | 10 ++++++--- 2 files changed, 36 insertions(+), 19 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qfsfileengine.cpp b/src/corelib/io/qfsfileengine.cpp index 387990ed79..5d3bef9e70 100644 --- a/src/corelib/io/qfsfileengine.cpp +++ b/src/corelib/io/qfsfileengine.cpp @@ -167,19 +167,21 @@ QFSFileEngine::QFSFileEngine(QFSFileEnginePrivate &dd) /*! \internal */ -bool QFSFileEngine::processOpenModeFlags(QIODevice::OpenMode *mode) +ProcessOpenModeResult processOpenModeFlags(QIODevice::OpenMode openMode) { - QIODevice::OpenMode &openMode = *mode; + ProcessOpenModeResult result; + result.ok = false; if ((openMode & QFile::NewOnly) && (openMode & QFile::ExistingOnly)) { qWarning("NewOnly and ExistingOnly are mutually exclusive"); - setError(QFile::OpenError, QLatin1String("NewOnly and ExistingOnly are mutually exclusive")); - return false; + result.error = QLatin1String("NewOnly and ExistingOnly are mutually exclusive"); + return result; } if ((openMode & QFile::ExistingOnly) && !(openMode & (QFile::ReadOnly | QFile::WriteOnly))) { qWarning("ExistingOnly must be specified alongside ReadOnly, WriteOnly, or ReadWrite"); - setError(QFile::OpenError, QLatin1String("ExistingOnly must be specified alongside ReadOnly, WriteOnly, or ReadWrite")); - return false; + result.error = QLatin1String( + "ExistingOnly must be specified alongside ReadOnly, WriteOnly, or ReadWrite"); + return result; } // Either Append or NewOnly implies WriteOnly @@ -190,7 +192,9 @@ bool QFSFileEngine::processOpenModeFlags(QIODevice::OpenMode *mode) if ((openMode & QFile::WriteOnly) && !(openMode & (QFile::ReadOnly | QFile::Append | QFile::NewOnly))) openMode |= QFile::Truncate; - return true; + result.ok = true; + result.openMode = openMode; + return result; } /*! @@ -234,16 +238,19 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode) return false; } - if (!processOpenModeFlags(&openMode)) + const ProcessOpenModeResult res = processOpenModeFlags(openMode); + if (!res.ok) { + setError(QFileDevice::OpenError, res.error); return false; + } - d->openMode = openMode; + d->openMode = res.openMode; d->lastFlushFailed = false; d->tried_stat = 0; d->fh = 0; d->fd = -1; - return d->nativeOpen(openMode); + return d->nativeOpen(d->openMode); } /*! @@ -262,17 +269,20 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode, FILE *fh, QFile::FileHand Q_D(QFSFileEngine); - if (!processOpenModeFlags(&openMode)) + const ProcessOpenModeResult res = processOpenModeFlags(openMode); + if (!res.ok) { + setError(QFileDevice::OpenError, res.error); return false; + } - d->openMode = openMode; + d->openMode = res.openMode; d->lastFlushFailed = false; d->closeFileHandle = (handleFlags & QFile::AutoCloseHandle); d->fileEntry.clear(); d->tried_stat = 0; d->fd = -1; - return d->openFh(openMode, fh); + return d->openFh(d->openMode, fh); } /*! @@ -321,10 +331,13 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode, int fd, QFile::FileHandle { Q_D(QFSFileEngine); - if (!processOpenModeFlags(&openMode)) + const ProcessOpenModeResult res = processOpenModeFlags(openMode); + if (!res.ok) { + setError(QFileDevice::OpenError, res.error); return false; + } - d->openMode = openMode; + d->openMode = res.openMode; d->lastFlushFailed = false; d->closeFileHandle = (handleFlags & QFile::AutoCloseHandle); d->fileEntry.clear(); @@ -332,7 +345,7 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode, int fd, QFile::FileHandle d->fd = -1; d->tried_stat = 0; - return d->openFd(openMode, fd); + return d->openFd(d->openMode, fd); } diff --git a/src/corelib/io/qfsfileengine_p.h b/src/corelib/io/qfsfileengine_p.h index 6b091a8eef..5231b4fe6f 100644 --- a/src/corelib/io/qfsfileengine_p.h +++ b/src/corelib/io/qfsfileengine_p.h @@ -61,6 +61,13 @@ QT_BEGIN_NAMESPACE +struct ProcessOpenModeResult { + bool ok; + QIODevice::OpenMode openMode; + QString error; +}; +inline Q_CORE_EXPORT ProcessOpenModeResult processOpenModeFlags(QIODevice::OpenMode mode); + class QFSFileEnginePrivate; class Q_CORE_EXPORT QFSFileEngine : public QAbstractFileEngine @@ -131,9 +138,6 @@ public: protected: QFSFileEngine(QFSFileEnginePrivate &dd); - -private: - inline bool processOpenModeFlags(QIODevice::OpenMode *mode); }; class Q_AUTOTEST_EXPORT QFSFileEnginePrivate : public QAbstractFileEnginePrivate -- cgit v1.2.3