From b3e55fbf4ec8b260d358640808ea459a5125049b Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C4=99drzej=20Nowacki?= Date: Wed, 18 Apr 2012 14:40:50 +0200 Subject: [PATCH] Do not assert when QVariant is constructed from an invalid type id That change also fix moduleForType() which was wrongly recognizing negative ids as belonging to Core. New tests were added. Change-Id: I40a5819effb32489a45937011980457387c9f8be Reviewed-by: Kent Hansen --- src/corelib/kernel/qmetatype_p.h | 2 +- src/corelib/kernel/qvariant.cpp | 5 +- src/widgets/kernel/qwidgetsvariant.cpp | 3 +- .../auto/corelib/kernel/qvariant/tst_qvariant.cpp | 63 ++++++++++++++++++++++ 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/corelib/kernel/qmetatype_p.h b/src/corelib/kernel/qmetatype_p.h index 2ab4a38..985cf71 100644 --- a/src/corelib/kernel/qmetatype_p.h +++ b/src/corelib/kernel/qmetatype_p.h @@ -60,7 +60,7 @@ QT_BEGIN_NAMESPACE namespace QModulesPrivate { enum Names { Core, Gui, Widgets, Unknown, ModulesCount /* ModulesCount has to be at the end */ }; -static inline int moduleForType(const int typeId) +static inline int moduleForType(const uint typeId) { if (typeId <= QMetaType::LastCoreType) return Core; diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 47c8709..296b845 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -87,7 +87,7 @@ class HandlersManager { static const QVariant::Handler *Handlers[QModulesPrivate::ModulesCount]; public: - const QVariant::Handler *operator[] (const int typeId) const + const QVariant::Handler *operator[] (const uint typeId) const { return Handlers[QModulesPrivate::moduleForType(typeId)]; } @@ -776,6 +776,7 @@ static void customConstruct(QVariant::Private *d, const void *copy) const QMetaType type(d->type); const uint size = type.sizeOf(); if (!size) { + qWarning("Trying to construct an instance of an invalid type, type id: %i", d->type); d->type = QVariant::Invalid; return; } @@ -1700,7 +1701,7 @@ void QVariant::load(QDataStream &s) return; } } - create(static_cast(typeId), 0); + create(typeId, 0); d.is_null = is_null; if (!isValid()) { diff --git a/src/widgets/kernel/qwidgetsvariant.cpp b/src/widgets/kernel/qwidgetsvariant.cpp index f6817ce..15935a5 100644 --- a/src/widgets/kernel/qwidgetsvariant.cpp +++ b/src/widgets/kernel/qwidgetsvariant.cpp @@ -62,7 +62,8 @@ static void construct(QVariant::Private *x, const void *copy) v_construct(x, copy); break; default: - Q_ASSERT(false); + qWarning("Trying to construct an instance of an invalid type, type id: %i", x->type); + x->type = QVariant::Invalid; return; } x->is_null = !copy; diff --git a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp index 6a6460d..91b56e4 100644 --- a/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp +++ b/tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp @@ -102,6 +102,8 @@ private slots: void constructor(); void copy_constructor(); + void constructor_invalid_data(); + void constructor_invalid(); void isNull(); void swap(); @@ -348,6 +350,67 @@ void tst_QVariant::constructor() QVERIFY(!var8.isNull()); } +void tst_QVariant::constructor_invalid_data() +{ + QTest::addColumn("typeId"); + + QTest::newRow("-1") << uint(-1); + QTest::newRow("-122234567") << uint(-122234567); + QTest::newRow("0xfffffffff") << uint(0xfffffffff); + QTest::newRow("LastCoreType + 1") << uint(QMetaType::LastCoreType + 1); + QVERIFY(!QMetaType::isRegistered(QMetaType::LastCoreType + 1)); + QTest::newRow("LastGuiType + 1") << uint(QMetaType::LastGuiType + 1); + QVERIFY(!QMetaType::isRegistered(QMetaType::LastGuiType + 1)); + QTest::newRow("LastWidgetsType + 1") << uint(QMetaType::LastWidgetsType + 1); + QVERIFY(!QMetaType::isRegistered(QMetaType::LastWidgetsType + 1)); +} + +struct MessageHandlerInvalidType +{ + MessageHandlerInvalidType() + : oldMsgHandler(qInstallMsgHandler(handler)) + { + ok = false; + } + + ~MessageHandlerInvalidType() + { + qInstallMsgHandler(oldMsgHandler); + } + + QtMsgHandler oldMsgHandler; + + static void handler(QtMsgType type, const char *txt) + { + QString msg = QString::fromLatin1(txt); + // uint(-1) can be platform dependent so we check only beginning of the message. + ok = msg.startsWith("Trying to construct an instance of an invalid type, type id:"); + QVERIFY2(ok, (QString::fromLatin1("Message is not started correctly: '") + msg + '\'').toLatin1().constData()); + } + static bool ok; +}; +bool MessageHandlerInvalidType::ok; + +void tst_QVariant::constructor_invalid() +{ + + QFETCH(uint, typeId); + { + MessageHandlerInvalidType msg; + QVariant variant(static_cast(typeId)); + QVERIFY(!variant.isValid()); + QVERIFY(variant.userType() == QMetaType::UnknownType); + QVERIFY(msg.ok); + } + { + MessageHandlerInvalidType msg; + QVariant variant(typeId, /* copy */ 0); + QVERIFY(!variant.isValid()); + QVERIFY(variant.userType() == QMetaType::UnknownType); + QVERIFY(msg.ok); + } +} + void tst_QVariant::copy_constructor() { QVariant var7(QVariant::Int); -- 2.7.4