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 <kent.hansen@nokia.com>

diff --git a/src/corelib/kernel/qmetatype_p.h b/src/corelib/kernel/qmetatype_p.h
index 2ab4a38..985cf71 100644 (file)
--- 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 (file)
--- 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<int>(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 (file)
--- 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<QSizePolicy>(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 (file)
--- 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<uint>("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<QVariant::Type>(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);