Fix QString and QByteArray reserve() and squeeze()

These functions should not take care not to unconditionally set the
capacityReserved private member, since the d may be referencing the
const shared_null or shared_empty which live in read-only memory.

The squeeze() methods check for ref > 1 instead of ref != 1 to prevent
detaching from the shared_null/shared_empty unnecessarily; the
shared_null/shared_empty ref count is -1, meaning squeeze() will never
detach from it.

Change-Id: Id3f1725a6f08b3a462343640a47bbe78f08ca7e7
Rubberstamped-by: Lars Knoll
Reviewed-on: http://codereview.qt-project.org/5454
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Lars Knoll <lars.knoll@nokia.com>

4 files changed, 58 insertions, 4 deletions

diff --git a/src/corelib/tools/qbytearray.h b/src/corelib/tools/qbytearray.h
index 3bb26ba21e..3ebeb3c340 100644
--- a/src/corelib/tools/qbytearray.h
+++ b/src/corelib/tools/qbytearray.h
@@ -439,10 +439,26 @@ inline int QByteArray::capacity() const
 { return d->alloc; }
 
 inline void QByteArray::reserve(int asize)
-{ if (d->ref != 1 || asize > int(d->alloc)) realloc(asize); d->capacityReserved = true; }
+{
+    if (d->ref != 1 || asize > int(d->alloc))
+        realloc(asize);
+
+    if (!d->capacityReserved) {
+        // cannot set unconditionally, since d could be the shared_null/shared_empty (which is const)
+        d->capacityReserved = true;
+    }
+}
 
 inline void QByteArray::squeeze()
-{ if (d->ref != 1 || d->size < int(d->alloc)) realloc(d->size); d->capacityReserved = false; }
+{
+    if (d->ref > 1 || d->size < int(d->alloc))
+        realloc(d->size);
+
+    if (d->capacityReserved) {
+        // cannot set unconditionally, since d could be the shared_null/shared_empty (which is const)
+        d->capacityReserved = false;
+    }
+}
 
 class Q_CORE_EXPORT QByteRef {
     QByteArray &a;
diff --git a/src/corelib/tools/qstring.h b/src/corelib/tools/qstring.h
index 209994de16..5009686d27 100644
--- a/src/corelib/tools/qstring.h
+++ b/src/corelib/tools/qstring.h
@@ -192,7 +192,7 @@ public:
 
     int capacity() const;
     inline void reserve(int size);
-    inline void squeeze() { if (d->size < int(d->alloc) || d->ref != 1) realloc(); d->capacityReserved = false;}
+    inline void squeeze();
 
     inline const QChar *unicode() const;
     inline QChar *data();
@@ -849,7 +849,29 @@ inline void QCharRef::setCell(uchar acell) { QChar(*this).setCell(acell); }
 
 inline QString::QString() : d(const_cast<Data *>(&shared_null.str)) {}
 inline QString::~QString() { if (!d->ref.deref()) free(d); }
-inline void QString::reserve(int asize) { if (d->ref != 1 || asize > int(d->alloc)) realloc(asize); d->capacityReserved = true;}
+
+inline void QString::reserve(int asize)
+{
+    if (d->ref != 1 || asize > int(d->alloc))
+        realloc(asize);
+
+    if (!d->capacityReserved) {
+        // cannot set unconditionally, since d could be the shared_null/shared_empty (which is const)
+        d->capacityReserved = true;
+    }
+}
+
+inline void QString::squeeze()
+{
+    if (d->ref > 1 || d->size < int(d->alloc))
+        realloc();
+
+    if (d->capacityReserved) {
+        // cannot set unconditionally, since d could be the shared_null/shared_empty (which is const)
+        d->capacityReserved = false;
+    }
+}
+
 inline QString &QString::setUtf16(const ushort *autf16, int asize)
 { return setUnicode(reinterpret_cast<const QChar *>(autf16), asize); }
 inline QCharRef QString::operator[](int i)
diff --git a/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp b/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp
index 5f036f37fe..dc0c8905fd 100644
--- a/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp
+++ b/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp
@@ -1536,6 +1536,12 @@ void tst_QByteArray::reserve()
         QVERIFY(capacity == qba.capacity());
         QVERIFY(data == qba.data());
     }
+
+    QByteArray nil1, nil2;
+    nil1.reserve(0);
+    nil2.squeeze();
+    nil1.squeeze();
+    nil2.reserve(0);
 }
 
 void tst_QByteArray::literals()
diff --git a/tests/auto/corelib/tools/qstring/tst_qstring.cpp b/tests/auto/corelib/tools/qstring/tst_qstring.cpp
index 5ef8d70ef6..dc5070c9c2 100644
--- a/tests/auto/corelib/tools/qstring/tst_qstring.cpp
+++ b/tests/auto/corelib/tools/qstring/tst_qstring.cpp
@@ -224,6 +224,8 @@ private slots:
 
     void toUpperLower_icu();
     void literals();
+
+    void reserve();
 };
 
 typedef QList<int> IntList;
@@ -5137,6 +5139,14 @@ void tst_QString::literals()
 #endif
 }
 
+void tst_QString::reserve()
+{
+    QString nil1, nil2;
+    nil1.reserve(0);
+    nil2.squeeze();
+    nil1.squeeze();
+    nil2.reserve(0);
+}
 
 QTEST_APPLESS_MAIN(tst_QString)