Don't crash by modifying read-only shared_null

Functions that modify the d-pointer must detach or otherwise take
measures to not modify the const, read-only shared_null.

The setSharable(bool) function takes care to detach when setting
sharable to false, but should avoid setting the sharable data member
unless d is not the shared null.

Similarly, QMap<Key, T>::setInsertInOrder() needs to detach if it is
shared with the shared_null (the logic has been updated to be the same
as setSharable()).

Change-Id: Ida5cb9818b86695f1b9f0264418b955c56424898
Reviewed-on: http://codereview.qt-project.org/5929
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>
Reviewed-by: Jan-Arve Sæther <jan-arve.saether@nokia.com>

diff --git a/src/corelib/tools/qhash.h b/src/corelib/tools/qhash.h
index 25cece4..cc120aa 100644 (file)
--- a/src/corelib/tools/qhash.h
+++ b/src/corelib/tools/qhash.h
@@ -291,7 +291,7 @@ public:
 
     inline void detach() { if (d->ref != 1) detach_helper(); }
     inline bool isDetached() const { return d->ref == 1; }
-    inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; }
+    inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QHashData::shared_null) d->sharable = sharable; }
     inline bool isSharedWith(const QHash<Key, T> &other) const { return d == other.d; }
 
     void clear();

diff --git a/src/corelib/tools/qlinkedlist.h b/src/corelib/tools/qlinkedlist.h
index bcad210..36cbc68 100644 (file)
--- a/src/corelib/tools/qlinkedlist.h
+++ b/src/corelib/tools/qlinkedlist.h
@@ -97,7 +97,7 @@ public:
     inline void detach()
     { if (d->ref != 1) detach_helper(); }
     inline bool isDetached() const { return d->ref == 1; }
-    inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; }
+    inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QLinkedListData::shared_null) d->sharable = sharable; }
     inline bool isSharedWith(const QLinkedList<T> &other) const { return d == other.d; }
 
     inline bool isEmpty() const { return d->size == 0; }

diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h
index e9e44db..5c8a58a 100644 (file)
--- a/src/corelib/tools/qlist.h
+++ b/src/corelib/tools/qlist.h
@@ -143,7 +143,7 @@ public:
     }
 
     inline bool isDetached() const { return d->ref == 1; }
-    inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; }
+    inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QListData::shared_null) d->sharable = sharable; }
     inline bool isSharedWith(const QList<T> &other) const { return d == other.d; }
 
     inline bool isEmpty() const { return p.isEmpty(); }

diff --git a/src/corelib/tools/qmap.h b/src/corelib/tools/qmap.h
index ff181d5..8beae03 100644 (file)
--- a/src/corelib/tools/qmap.h
+++ b/src/corelib/tools/qmap.h
@@ -204,9 +204,9 @@ public:
 
     inline void detach() { if (d->ref != 1) detach_helper(); }
     inline bool isDetached() const { return d->ref == 1; }
-    inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; }
+    inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QMapData::shared_null) d->sharable = sharable; }
     inline bool isSharedWith(const QMap<Key, T> &other) const { return d == other.d; }
-    inline void setInsertInOrder(bool ordered) { d->insertInOrder = ordered; }
+    inline void setInsertInOrder(bool ordered) { if (ordered) detach(); if (d != &QMapData::shared_null) d->insertInOrder = ordered; }
 
     void clear();
 

diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h
index 34d1ed3..eab9311 100644 (file)
--- a/src/corelib/tools/qvector.h
+++ b/src/corelib/tools/qvector.h
@@ -146,7 +146,7 @@ public:
 
     inline void detach() { if (d->ref != 1) detach_helper(); }
     inline bool isDetached() const { return d->ref == 1; }
-    inline void setSharable(bool sharable) { if (!sharable) detach(); d->sharable = sharable; }
+    inline void setSharable(bool sharable) { if (!sharable) detach(); if (d != &QVectorData::shared_null) d->sharable = sharable; }
     inline bool isSharedWith(const QVector<T> &other) const { return d == other.d; }
 
     inline T *data() { detach(); return p->array; }

diff --git a/tests/auto/corelib/tools/qhash/tst_qhash.cpp b/tests/auto/corelib/tools/qhash/tst_qhash.cpp
index 16fb123..fc66962 100644 (file)
--- a/tests/auto/corelib/tools/qhash/tst_qhash.cpp
+++ b/tests/auto/corelib/tools/qhash/tst_qhash.cpp
@@ -79,6 +79,8 @@ private slots:
     void iterators(); // sligthly modified from tst_QMap
     void keys_values_uniqueKeys(); // slightly modified from tst_QMap
     void noNeedlessRehashes();
+
+    void const_shared_null();
 };
 
 struct Foo {
@@ -1237,5 +1239,16 @@ void tst_QHash::noNeedlessRehashes()
     }
 }
 
+void tst_QHash::const_shared_null()
+{
+    QHash<int, QString> hash1;
+    hash1.setSharable(false);
+    QVERIFY(hash1.isDetached());
+
+    QHash<int, QString> hash2;
+    hash2.setSharable(true);
+    QVERIFY(!hash2.isDetached());
+}
+
 QTEST_APPLESS_MAIN(tst_QHash)
 #include "tst_qhash.moc"

diff --git a/tests/auto/corelib/tools/qlist/tst_qlist.cpp b/tests/auto/corelib/tools/qlist/tst_qlist.cpp
index 3901b6f..3d06291 100644 (file)
--- a/tests/auto/corelib/tools/qlist/tst_qlist.cpp
+++ b/tests/auto/corelib/tools/qlist/tst_qlist.cpp
@@ -91,6 +91,8 @@ private slots:
     void testOperators() const;
 
     void initializeList() const;
+
+    void const_shared_null() const;
 };
 
 void tst_QList::length() const
@@ -688,5 +690,16 @@ void tst_QList::initializeList() const
 #endif
 }
 
+void tst_QList::const_shared_null() const
+{
+    QList<int> list1;
+    list1.setSharable(false);
+    QVERIFY(list1.isDetached());
+
+    QList<int> list2;
+    list2.setSharable(true);
+    QVERIFY(!list2.isDetached());
+}
+
 QTEST_APPLESS_MAIN(tst_QList)
 #include "tst_qlist.moc"

diff --git a/tests/auto/corelib/tools/qmap/tst_qmap.cpp b/tests/auto/corelib/tools/qmap/tst_qmap.cpp
index 141e693..2b4cbe9 100644 (file)
--- a/tests/auto/corelib/tools/qmap/tst_qmap.cpp
+++ b/tests/auto/corelib/tools/qmap/tst_qmap.cpp
@@ -80,6 +80,8 @@ private slots:
     void iterators();
     void keys_values_uniqueKeys();
     void qmultimap_specific();
+
+    void const_shared_null();
 };
 
 tst_QMap::tst_QMap()
@@ -871,5 +873,20 @@ void tst_QMap::qmultimap_specific()
     }
 }
 
+void tst_QMap::const_shared_null()
+{
+    QMap<int, QString> map1;
+    map1.setSharable(false);
+    QVERIFY(map1.isDetached());
+
+    QMap<int, QString> map2;
+    map2.setSharable(true);
+    QVERIFY(!map2.isDetached());
+
+    QMap<int, QString> map3;
+    map3.setInsertInOrder(true);
+    map3.setInsertInOrder(false);
+}
+
 QTEST_APPLESS_MAIN(tst_QMap)
 #include "tst_qmap.moc"

diff --git a/tests/auto/corelib/tools/qvector/tst_qvector.cpp b/tests/auto/corelib/tools/qvector/tst_qvector.cpp
index 4cc2bc4..2b615bc 100644 (file)
--- a/tests/auto/corelib/tools/qvector/tst_qvector.cpp
+++ b/tests/auto/corelib/tools/qvector/tst_qvector.cpp
@@ -92,6 +92,8 @@ private slots:
     void QTBUG11763_data();
     void QTBUG11763();
     void initializeList();
+
+    void const_shared_null();
 };
 
 void tst_QVector::constructors() const
@@ -940,5 +942,16 @@ void tst_QVector::initializeList()
 #endif
 }
 
+void tst_QVector::const_shared_null()
+{
+    QVector<int> v1;
+    v1.setSharable(false);
+    QVERIFY(v1.isDetached());
+
+    QVector<int> v2;
+    v2.setSharable(true);
+    QVERIFY(!v2.isDetached());
+}
+
 QTEST_APPLESS_MAIN(tst_QVector)
 #include "tst_qvector.moc"