1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QStateMachine: replace a QHash key involving a QPointer

Browse Source 
Using QPointers (or any type that makes a QPointer part of its identity) as a key
in any associative container is wrong. They get externally set to nullptr,
violating the associative container's class invariants, which could lead to
data corruption, even though bucket-based hash implementations are less susceptible
than binary trees.

To fix, write a new class that acts much like the old QPair<QPointer<>,QByteArray>,
but uses the QPointer only as a guard, not as part of its identity. To preseve
identity, also saves the naked pointer originally passed and uses that for op==
and qHash().

Change-Id: I4fa5a6bf86bad8fe7f5abe53d7c7f3ad3754d8d6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Marc Mutz 2015-01-08 14:57:09 +01:00
parent 65eb573926
commit 3b9629e8bd
2 changed files with 22 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/corelib/statemachine/qstatemachine.cpp
View File

@ -947,14 +947,15 @@ QList<QPropertyAssignment> QStateMachinePrivate::restorablesToPropertyList(const
QList<QPropertyAssignment> result; QList<QPropertyAssignment> result;
QHash<RestorableId, QVariant>::const_iterator it; QHash<RestorableId, QVariant>::const_iterator it;
for (it = restorables.constBegin(); it != restorables.constEnd(); ++it) { for (it = restorables.constBegin(); it != restorables.constEnd(); ++it) {
if (!it.key().first) { const RestorableId &id = it.key();
if (!id.object()) {
// Property object was deleted // Property object was deleted
continue; continue;
} }
#ifdef QSTATEMACHINE_RESTORE_PROPERTIES_DEBUG #ifdef QSTATEMACHINE_RESTORE_PROPERTIES_DEBUG
qDebug() << q_func() << ": restoring" << it.key().first << it.key().second << "to" << it.value(); qDebug() << q_func() << ": restoring" << id.object() << id.proertyName() << "to" << it.value();
#endif #endif
result.append(QPropertyAssignment(it.key().first, it.key().second, it.value(), /*explicitlySet=*/false)); result.append(QPropertyAssignment(id.object(), id.propertyName(), it.value(), /*explicitlySet=*/false));
} }
return result; return result;
} }

19
src/corelib/statemachine/qstatemachine_p.h
View File

@ -189,7 +189,24 @@ public:
void cancelAllDelayedEvents(); void cancelAllDelayedEvents();
#ifndef QT_NO_PROPERTIES #ifndef QT_NO_PROPERTIES
typedef QPair<QPointer<QObject>, QByteArray> RestorableId; class RestorableId {
QPointer<QObject> guard;
QObject *obj;
QByteArray prop;
// two overloads because friends can't have default arguments
friend uint qHash(const RestorableId &key, uint seed) Q_DECL_NOEXCEPT_EXPR(noexcept(std::declval<QByteArray>()))
{ return qHash(qMakePair(key.obj, key.prop), seed); }
friend uint qHash(const RestorableId &key) Q_DECL_NOEXCEPT_EXPR(noexcept(qHash(key, 0U)))
{ return qHash(key, 0U); }
friend bool operator==(const RestorableId &lhs, const RestorableId &rhs) Q_DECL_NOTHROW
{ return lhs.obj == rhs.obj && lhs.prop == rhs.prop; }
friend bool operator!=(const RestorableId &lhs, const RestorableId &rhs) Q_DECL_NOTHROW
{ return !operator==(lhs, rhs); }
public:
explicit RestorableId(QObject *o, QByteArray p) Q_DECL_NOTHROW : guard(o), obj(o), prop(qMove(p)) {}
QObject *object() const Q_DECL_NOTHROW { return guard; }
QByteArray propertyName() const Q_DECL_NOTHROW { return prop; }
};
QHash<QAbstractState*, QHash<RestorableId, QVariant> > registeredRestorablesForState; QHash<QAbstractState*, QHash<RestorableId, QVariant> > registeredRestorablesForState;
bool hasRestorable(QAbstractState *state, QObject *object, const QByteArray &propertyName) const; bool hasRestorable(QAbstractState *state, QObject *object, const QByteArray &propertyName) const;
QVariant savedValueForRestorable(const QList<QAbstractState*> &exitedStates_sorted, QVariant savedValueForRestorable(const QList<QAbstractState*> &exitedStates_sorted,