diff options
author | Olivier Goffart <ogoffart@woboq.com> | 2017-02-27 12:13:13 +0100 |
---|---|---|
committer | Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com> | 2017-03-06 18:32:28 +0000 |
commit | 6797570a59529b90b5a28923825b56703173fa56 (patch) | |
tree | c8fed7fd055446518bf1ceee7bf71a7d577b9cee /src/corelib/thread | |
parent | a170c974a53bde5c6027d3cb25311b819678c546 (diff) |
Fix UB in QFutureInterface: invalid casts from ResultStoreBase to ResultStore<>
ResultStore never actually exists, only ResutStoreBase does. So casting to
ResultStore<T> and calling its member functions is UB. Put the type dependent
function as template member functions within ResultStoreBase and so we don't
need QtPrivate::ResultStore anymore.
Same goes for the iterator.
Change-Id: I739b9d234ba2238977863df77fde3a4471a9abd2
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Diffstat (limited to 'src/corelib/thread')
-rw-r--r-- | src/corelib/thread/qfutureinterface.h | 28 | ||||
-rw-r--r-- | src/corelib/thread/qresultstore.cpp | 6 | ||||
-rw-r--r-- | src/corelib/thread/qresultstore.h | 64 |
3 files changed, 33 insertions, 65 deletions
diff --git a/src/corelib/thread/qfutureinterface.h b/src/corelib/thread/qfutureinterface.h index 559d26e231..7b12f51e3e 100644 --- a/src/corelib/thread/qfutureinterface.h +++ b/src/corelib/thread/qfutureinterface.h @@ -159,7 +159,7 @@ public: ~QFutureInterface() { if (!derefT()) - resultStore().clear(); + resultStoreBase().template clear<T>(); } static QFutureInterface canceledResult() @@ -169,7 +169,7 @@ public: { other.refT(); if (!derefT()) - resultStore().clear(); + resultStoreBase().template clear<T>(); QFutureInterfaceBase::operator=(other); return *this; } @@ -184,11 +184,6 @@ public: inline const T &resultReference(int index) const; inline const T *resultPointer(int index) const; inline QList<T> results(); -private: - QtPrivate::ResultStore<T> &resultStore() - { return static_cast<QtPrivate::ResultStore<T> &>(resultStoreBase()); } - const QtPrivate::ResultStore<T> &resultStore() const - { return static_cast<const QtPrivate::ResultStore<T> &>(resultStoreBase()); } }; template <typename T> @@ -199,15 +194,14 @@ inline void QFutureInterface<T>::reportResult(const T *result, int index) return; } - QtPrivate::ResultStore<T> &store = resultStore(); - + QtPrivate::ResultStoreBase &store = resultStoreBase(); if (store.filterMode()) { const int resultCountBefore = store.count(); - store.addResult(index, result); + store.addResult<T>(index, result); this->reportResultsReady(resultCountBefore, resultCountBefore + store.count()); } else { - const int insertIndex = store.addResult(index, result); + const int insertIndex = store.addResult<T>(index, result); this->reportResultsReady(insertIndex, insertIndex + 1); } } @@ -226,7 +220,7 @@ inline void QFutureInterface<T>::reportResults(const QVector<T> &_results, int b return; } - QtPrivate::ResultStore<T> &store = resultStore(); + auto &store = resultStoreBase(); if (store.filterMode()) { const int resultCountBefore = store.count(); @@ -250,14 +244,14 @@ template <typename T> inline const T &QFutureInterface<T>::resultReference(int index) const { QMutexLocker lock(mutex()); - return resultStore().resultAt(index).value(); + return resultStoreBase().resultAt(index).template value<T>(); } template <typename T> inline const T *QFutureInterface<T>::resultPointer(int index) const { QMutexLocker lock(mutex()); - return resultStore().resultAt(index).pointer(); + return resultStoreBase().resultAt(index).template pointer<T>(); } template <typename T> @@ -272,9 +266,9 @@ inline QList<T> QFutureInterface<T>::results() QList<T> res; QMutexLocker lock(mutex()); - QtPrivate::ResultIterator<T> it = resultStore().begin(); - while (it != resultStore().end()) { - res.append(it.value()); + QtPrivate::ResultIteratorBase it = resultStoreBase().begin(); + while (it != resultStoreBase().end()) { + res.append(it.value<T>()); ++it; } diff --git a/src/corelib/thread/qresultstore.cpp b/src/corelib/thread/qresultstore.cpp index aa7ec02d6e..9a6fcec678 100644 --- a/src/corelib/thread/qresultstore.cpp +++ b/src/corelib/thread/qresultstore.cpp @@ -98,6 +98,12 @@ bool ResultIteratorBase::canIncrementVectorIndex() const ResultStoreBase::ResultStoreBase() : insertIndex(0), resultCount(0), m_filterMode(false), filteredResults(0) { } +ResultStoreBase::~ResultStoreBase() +{ + // QFutureInterface's dtor must delete the contents of m_results. + Q_ASSERT(m_results.isEmpty()); +} + void ResultStoreBase::setFilterMode(bool enable) { m_filterMode = enable; diff --git a/src/corelib/thread/qresultstore.h b/src/corelib/thread/qresultstore.h index 56cfcb6ed6..be9f632557 100644 --- a/src/corelib/thread/qresultstore.h +++ b/src/corelib/thread/qresultstore.h @@ -93,20 +93,14 @@ public: protected: QMap<int, ResultItem>::const_iterator mapIterator; int m_vectorIndex; -}; - -template <typename T> -class ResultIterator : public ResultIteratorBase -{ public: - ResultIterator(const ResultIteratorBase &base) - : ResultIteratorBase(base) { } - + template <typename T> const T &value() const { - return *pointer(); + return *pointer<T>(); } + template <typename T> const T *pointer() const { if (mapIterator.value().isVector()) @@ -130,7 +124,7 @@ public: ResultIteratorBase resultAt(int index) const; bool contains(int index) const; int count() const; - virtual ~ResultStoreBase() { } + virtual ~ResultStoreBase(); protected: int insertResultItem(int index, ResultItem &resultItem); @@ -147,64 +141,44 @@ protected: QMap<int, ResultItem> pendingResults; int filteredResults; -}; - -template <typename T> -class ResultStore : public ResultStoreBase -{ public: - ResultStore() { } - - ResultStore(const ResultStoreBase &base) - : ResultStoreBase(base) { } - - int addResult(int index, const T *result) + template <typename T> + int addResult(int index, const T *result) { if (result == 0) - return ResultStoreBase::addResult(index, result); + return addResult(index, static_cast<void *>(nullptr)); else - return ResultStoreBase::addResult(index, new T(*result)); + return addResult(index, static_cast<void *>(new T(*result))); } + template <typename T> int addResults(int index, const QVector<T> *results) { - return ResultStoreBase::addResults(index, new QVector<T>(*results), results->count(), results->count()); + return addResults(index, new QVector<T>(*results), results->count(), results->count()); } + template <typename T> int addResults(int index, const QVector<T> *results, int totalCount) { if (m_filterMode == true && results->count() != totalCount && 0 == results->count()) - return ResultStoreBase::addResults(index, 0, 0, totalCount); + return addResults(index, 0, 0, totalCount); else - return ResultStoreBase::addResults(index, new QVector<T>(*results), results->count(), totalCount); + return addResults(index, new QVector<T>(*results), results->count(), totalCount); } int addCanceledResult(int index) { - return addResult(index, 0); + return addResult(index, static_cast<void *>(nullptr)); } + template <typename T> int addCanceledResults(int index, int _count) { QVector<T> empty; return addResults(index, &empty, _count); } - ResultIterator<T> begin() const - { - return static_cast<ResultIterator<T> >(ResultStoreBase::begin()); - } - - ResultIterator<T> end() const - { - return static_cast<ResultIterator<T> >(ResultStoreBase::end()); - } - - ResultIterator<T> resultAt(int index) const - { - return static_cast<ResultIterator<T> >(ResultStoreBase::resultAt(index)); - } - + template <typename T> void clear() { QMap<int, ResultItem>::const_iterator mapIterator = m_results.constBegin(); @@ -218,12 +192,6 @@ public: resultCount = 0; m_results.clear(); } - - ~ResultStore() - { - clear(); - } - }; } // namespace QtPrivate |