From: João Abecasis <joao.abecasis@nokia.com>
Date: Fri, 24 Aug 2012 08:53:58 +0000 (+0200)
Subject: Don't manipulate immutable data
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=975ebdc4ab77d1c0d8bc91e393eca7095a5d8742;p=profile%2Fivi%2Fqtbase.git

Don't manipulate immutable data

QArrayData can point to data it does not own (cf. fromRawData()), which
shouldn't be modified. Not even upon destruction, as this data can live
in Read-Only memory or be otherwise shared outside the QArrayData realm.

Change-Id: I8bdf3050a17802fb003b77d5f543fe31769a7710
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Andreas Hartmetz <ahartmetz@gmail.com>
---

diff --git a/src/corelib/tools/qarraydatapointer.h b/src/corelib/tools/qarraydatapointer.h
index 65ebc29..724edca 100644
--- a/src/corelib/tools/qarraydatapointer.h
+++ b/src/corelib/tools/qarraydatapointer.h
@@ -115,7 +115,8 @@ public:
     ~QArrayDataPointer()
     {
         if (!d->ref.deref()) {
-            (*this)->destroyAll();
+            if (d->isMutable())
+                (*this)->destroyAll();
             Data::deallocate(d);
         }
     }
diff --git a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
index dea777d..437c8e2 100644
--- a/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
+++ b/tests/auto/corelib/tools/qarraydata/tst_qarraydata.cpp
@@ -84,6 +84,7 @@ private slots:
     void arrayOps2();
     void setSharable_data();
     void setSharable();
+    void fromRawData_data();
     void fromRawData();
     void literals();
     void variadicLiterals();
@@ -1421,53 +1422,106 @@ void tst_QArrayData::setSharable()
     QVERIFY(array->ref.isSharable());
 }
 
-void tst_QArrayData::fromRawData()
+struct ResetOnDtor
+{
+    ResetOnDtor()
+        : value_()
+    {
+    }
+
+    ResetOnDtor(int value)
+        : value_(value)
+    {
+    }
+
+    ~ResetOnDtor()
+    {
+        value_ = 0;
+    }
+
+    int value_;
+};
+
+bool operator==(const ResetOnDtor &lhs, const ResetOnDtor &rhs)
+{
+    return lhs.value_ == rhs.value_;
+}
+
+template <class T>
+void fromRawData_impl()
 {
-    static const int array[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 };
+    static const T array[] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11 };
 
     {
         // Default: Immutable, sharable
-        SimpleVector<int> raw = SimpleVector<int>::fromRawData(array,
+        SimpleVector<T> raw = SimpleVector<T>::fromRawData(array,
                 sizeof(array)/sizeof(array[0]), QArrayData::Default);
 
         QCOMPARE(raw.size(), size_t(11));
-        QCOMPARE((const int *)raw.constBegin(), array);
-        QCOMPARE((const int *)raw.constEnd(), (const int *)(array + sizeof(array)/sizeof(array[0])));
+        QCOMPARE((const T *)raw.constBegin(), array);
+        QCOMPARE((const T *)raw.constEnd(), (const T *)(array + sizeof(array)/sizeof(array[0])));
 
         QVERIFY(!raw.isShared());
-        QVERIFY(SimpleVector<int>(raw).isSharedWith(raw));
+        QVERIFY(SimpleVector<T>(raw).isSharedWith(raw));
         QVERIFY(!raw.isShared());
 
         // Detach
-        QCOMPARE(raw.back(), 11);
-        QVERIFY((const int *)raw.constBegin() != array);
+        QCOMPARE(raw.back(), T(11));
+        QVERIFY((const T *)raw.constBegin() != array);
     }
 
     {
         // Immutable, unsharable
-        SimpleVector<int> raw = SimpleVector<int>::fromRawData(array,
+        SimpleVector<T> raw = SimpleVector<T>::fromRawData(array,
                 sizeof(array)/sizeof(array[0]), QArrayData::Unsharable);
 
         QCOMPARE(raw.size(), size_t(11));
-        QCOMPARE((const int *)raw.constBegin(), array);
-        QCOMPARE((const int *)raw.constEnd(), (const int *)(array + sizeof(array)/sizeof(array[0])));
+        QCOMPARE((const T *)raw.constBegin(), array);
+        QCOMPARE((const T *)raw.constEnd(), (const T *)(array + sizeof(array)/sizeof(array[0])));
 
-        SimpleVector<int> copy(raw);
+        SimpleVector<T> copy(raw);
         QVERIFY(!copy.isSharedWith(raw));
         QVERIFY(!raw.isShared());
 
         QCOMPARE(copy.size(), size_t(11));
 
-        for (size_t i = 0; i < 11; ++i)
+        for (size_t i = 0; i < 11; ++i) {
             QCOMPARE(const_(copy)[i], const_(raw)[i]);
+            QCOMPARE(const_(copy)[i], T(i + 1));
+        }
 
         QCOMPARE(raw.size(), size_t(11));
-        QCOMPARE((const int *)raw.constBegin(), array);
-        QCOMPARE((const int *)raw.constEnd(), (const int *)(array + sizeof(array)/sizeof(array[0])));
+        QCOMPARE((const T *)raw.constBegin(), array);
+        QCOMPARE((const T *)raw.constEnd(), (const T *)(array + sizeof(array)/sizeof(array[0])));
 
         // Detach
-        QCOMPARE(raw.back(), 11);
-        QVERIFY((const int *)raw.constBegin() != array);
+        QCOMPARE(raw.back(), T(11));
+        QVERIFY((const T *)raw.constBegin() != array);
+    }
+}
+
+void tst_QArrayData::fromRawData_data()
+{
+    QTest::addColumn<int>("type");
+
+    QTest::newRow("int") << 0;
+    QTest::newRow("ResetOnDtor") << 1;
+}
+void tst_QArrayData::fromRawData()
+{
+    QFETCH(int, type);
+
+    switch (type)
+    {
+        case 0:
+            fromRawData_impl<int>();
+            break;
+        case 1:
+            fromRawData_impl<ResetOnDtor>();
+            break;
+
+        default:
+            QFAIL("Unexpected type data");
     }
 }