From a959f34d716f42925b22d42838e7a4b97e415c69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Sun, 1 Apr 2012 01:05:47 +0200 Subject: Clean up constructors for "statics" in QString and QByteArray There were two constuctors offering essentially the same functionality. One taking the QStatic*Data struct, the other what essentially amounts to a pointer wrapper of that struct. The former was dropped and the latter untemplatized and kept, as that is the most generic and widely applicable. The template parameter in the wrapper was not very useful as it essentially duplicated information that already maintained in the struct, and there were no consistency checks to ensure they were in sync. In this case, using a wrapper is preferred over the use of naked pointers both as a way to make explicit the transfer of ownership as well as to avoid unintended conversions. By using the reference count (even if only by calling deref() in the destructor), QByteArray and QString must own their Data pointers. Const qualification was dropped from the member variable in these wrappers as it causes some compilers to emit warnings on the lack of constructors, and because it isn't needed there. To otherwise reduce noise, QStatic*Data gained a member function to directly access the const_cast'ed naked pointer. This plays nicely with the above constructor. Its use also allows us to do further changes in the QStatic*Data structs with fewer changes in remaining code. The function has an assert on isStatic(), to ensure it is not inadvertently used with data that requires ref-count operations. With this change, the need for the private constructor taking a naked Q*Data pointer is obviated and that was dropped too. In updating QStringBuilder's QConcatenable specializations I noticed they were broken (using data, instead of data()), so a test was added to avoid this happening again in the future. An unnecessary ref-count increment in QByteArray::clear was also dropped. Change-Id: I9b92fbaae726ab9807837e83d0d19812bf7db5ab Reviewed-by: Thiago Macieira --- src/corelib/json/qjsonvalue.cpp | 3 +- src/corelib/kernel/qmetaobject.cpp | 3 +- src/corelib/tools/qbytearray.cpp | 30 ++++++++------- src/corelib/tools/qbytearray.h | 37 ++++++++++-------- src/corelib/tools/qstring.cpp | 31 ++++++++------- src/corelib/tools/qstring.h | 44 +++++++++++++--------- src/corelib/tools/qstringbuilder.h | 22 +++++------ .../corelib/tools/qbytearray/tst_qbytearray.cpp | 10 ++--- .../qstringbuilder1/stringbuilder.cpp | 30 +++++++++++++++ 9 files changed, 134 insertions(+), 76 deletions(-) diff --git a/src/corelib/json/qjsonvalue.cpp b/src/corelib/json/qjsonvalue.cpp index 2eedef67ce..a5234945a8 100644 --- a/src/corelib/json/qjsonvalue.cpp +++ b/src/corelib/json/qjsonvalue.cpp @@ -400,7 +400,8 @@ QString QJsonValue::toString() const if (t != String) return QString(); stringData->ref.ref(); // the constructor below doesn't add a ref. - return QString(*(const QStaticStringData<1> *)stringData); + QStringDataPtr holder = { stringData }; + return QString(holder); } /*! diff --git a/src/corelib/kernel/qmetaobject.cpp b/src/corelib/kernel/qmetaobject.cpp index 75dbb49c81..a93046dcd1 100644 --- a/src/corelib/kernel/qmetaobject.cpp +++ b/src/corelib/kernel/qmetaobject.cpp @@ -159,7 +159,8 @@ static inline const QByteArrayData &stringData(const QMetaObject *mo, int index) static inline QByteArray toByteArray(const QByteArrayData &d) { - return QByteArray(reinterpret_cast &>(d)); + QByteArrayDataPtr holder = { const_cast(&d) }; + return QByteArray(holder); } static inline const char *rawStringData(const QMetaObject *mo, int index) diff --git a/src/corelib/tools/qbytearray.cpp b/src/corelib/tools/qbytearray.cpp index 574153fdaf..71244e0eab 100644 --- a/src/corelib/tools/qbytearray.cpp +++ b/src/corelib/tools/qbytearray.cpp @@ -584,7 +584,10 @@ QByteArray qUncompress(const uchar* data, int nbytes) d->offset = sizeof(QByteArrayData); d->data()[len] = 0; - return QByteArray(d.take(), 0, 0); + { + QByteArrayDataPtr dataPtr = { d.take() }; + return QByteArray(dataPtr); + } case Z_MEM_ERROR: qWarning("qUncompress: Z_MEM_ERROR: Not enough memory"); @@ -912,9 +915,9 @@ QByteArray &QByteArray::operator=(const char *str) { Data *x; if (!str) { - x = const_cast(&shared_null.ba); + x = shared_null.data_ptr(); } else if (!*str) { - x = const_cast(&shared_empty.ba); + x = shared_empty.data_ptr(); } else { int len = strlen(str); if (d->ref.isShared() || len > int(d->alloc) || (len < d->size && len < int(d->alloc) >> 1)) @@ -1320,12 +1323,12 @@ void QByteArray::chop(int n) QByteArray::QByteArray(const char *data, int size) { if (!data) { - d = const_cast(&shared_null.ba); + d = shared_null.data_ptr(); } else { if (size < 0) size = strlen(data); if (!size) { - d = const_cast(&shared_empty.ba); + d = shared_empty.data_ptr(); } else { d = static_cast(malloc(sizeof(Data) + size + 1)); Q_CHECK_PTR(d); @@ -1350,7 +1353,7 @@ QByteArray::QByteArray(const char *data, int size) QByteArray::QByteArray(int size, char ch) { if (size <= 0) { - d = const_cast(&shared_empty.ba); + d = shared_empty.data_ptr(); } else { d = static_cast(malloc(sizeof(Data) + size + 1)); Q_CHECK_PTR(d); @@ -1406,7 +1409,7 @@ void QByteArray::resize(int size) } if (size == 0 && !d->capacityReserved) { - Data *x = const_cast(&shared_empty.ba); + Data *x = shared_empty.data_ptr(); if (!d->ref.deref()) free(d); d = x; @@ -2728,8 +2731,7 @@ void QByteArray::clear() { if (!d->ref.deref()) free(d); - d = const_cast(&shared_null.ba); - d->ref.ref(); + d = shared_null.data_ptr(); } #if !defined(QT_NO_DATASTREAM) || (defined(QT_BOOTSTRAPPED) && !defined(QT_BUILD_QMAKE)) @@ -3171,7 +3173,8 @@ QByteArray QByteArray::trimmed() const } int l = end - start + 1; if (l <= 0) { - return QByteArray(const_cast(&shared_empty.ba), 0, 0); + QByteArrayDataPtr empty = { shared_empty.data_ptr() }; + return QByteArray(empty); } return QByteArray(s+start, l); } @@ -3878,9 +3881,9 @@ QByteArray QByteArray::fromRawData(const char *data, int size) { Data *x; if (!data) { - x = const_cast(&shared_null.ba); + x = shared_null.data_ptr(); } else if (!size) { - x = const_cast(&shared_empty.ba); + x = shared_empty.data_ptr(); } else { x = static_cast(malloc(sizeof(Data) + 1)); Q_CHECK_PTR(x); @@ -3890,7 +3893,8 @@ QByteArray QByteArray::fromRawData(const char *data, int size) x->capacityReserved = false; x->offset = data - reinterpret_cast(x); } - return QByteArray(x, 0, 0); + QByteArrayDataPtr dataPtr = { x }; + return QByteArray(dataPtr); } /*! diff --git a/src/corelib/tools/qbytearray.h b/src/corelib/tools/qbytearray.h index 31fa462ca4..2065c8fe91 100644 --- a/src/corelib/tools/qbytearray.h +++ b/src/corelib/tools/qbytearray.h @@ -136,21 +136,31 @@ template struct QStaticByteArrayData { QByteArrayData ba; char data[N + 1]; + + QByteArrayData *data_ptr() const + { + Q_ASSERT(ba.ref.isStatic()); + return const_cast(&ba); + } }; -template struct QStaticByteArrayDataPtr +struct QByteArrayDataPtr { - const QStaticByteArrayData *ptr; + QByteArrayData *ptr; }; #if defined(Q_COMPILER_LAMBDA) -# define QByteArrayLiteral(str) ([]() -> QStaticByteArrayDataPtr { \ + +# define QByteArrayLiteral(str) \ + ([]() -> QByteArrayDataPtr { \ enum { Size = sizeof(str) - 1 }; \ static const QStaticByteArrayData qbytearray_literal = \ { { Q_REFCOUNT_INITIALIZE_STATIC, Size, 0, 0, sizeof(QByteArrayData) }, str }; \ - QStaticByteArrayDataPtr holder = { &qbytearray_literal }; \ - return holder; }()) + QByteArrayDataPtr holder = { qbytearray_literal.data_ptr() }; \ + return holder; \ + }()) \ + /**/ #elif defined(Q_CC_GNU) // We need to create a QByteArrayData in the .rodata section of memory @@ -162,8 +172,11 @@ template struct QStaticByteArrayDataPtr enum { Size = sizeof(str) - 1 }; \ static const QStaticByteArrayData qbytearray_literal = \ { { Q_REFCOUNT_INITIALIZE_STATIC, Size, 0, 0, sizeof(QByteArrayData) }, str }; \ - QStaticByteArrayDataPtr holder = { &qbytearray_literal }; \ - holder; }) + QByteArrayDataPtr holder = { qbytearray_literal.data_ptr() }; \ + holder; \ + }) \ + /**/ + #endif #ifndef QByteArrayLiteral @@ -377,19 +390,13 @@ public: int length() const { return d->size; } bool isNull() const; - template - inline QByteArray(const QStaticByteArrayData &dd) - : d(const_cast(&dd.ba)) {} - template - Q_DECL_CONSTEXPR inline QByteArray(QStaticByteArrayDataPtr dd) - : d(const_cast(&dd.ptr->ba)) {} + Q_DECL_CONSTEXPR inline QByteArray(QByteArrayDataPtr dd) : d(dd.ptr) {} private: operator QNoImplicitBoolCast() const; static const QStaticByteArrayData<1> shared_null; static const QStaticByteArrayData<1> shared_empty; Data *d; - QByteArray(Data *dd, int /*dummy*/, int /*dummy*/) : d(dd) {} void realloc(int alloc); void expand(int i); QByteArray nulTerminated() const; @@ -402,7 +409,7 @@ public: inline DataPtr &data_ptr() { return d; } }; -inline QByteArray::QByteArray(): d(const_cast(&shared_null.ba)) { } +inline QByteArray::QByteArray(): d(shared_null.data_ptr()) { } inline QByteArray::~QByteArray() { if (!d->ref.deref()) free(d); } inline int QByteArray::size() const { return d->size; } diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index bb66fdbdec..3bd2deee66 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -1050,7 +1050,7 @@ int QString::toUcs4_helper(const ushort *uc, int length, uint *out) QString::QString(const QChar *unicode, int size) { if (!unicode) { - d = const_cast(&shared_null.str); + d = shared_null.data_ptr(); } else { if (size < 0) { size = 0; @@ -1058,7 +1058,7 @@ QString::QString(const QChar *unicode, int size) ++size; } if (!size) { - d = const_cast(&shared_empty.str); + d = shared_empty.data_ptr(); } else { d = (Data*) ::malloc(sizeof(Data)+(size+1)*sizeof(QChar)); Q_CHECK_PTR(d); @@ -1082,7 +1082,7 @@ QString::QString(const QChar *unicode, int size) QString::QString(int size, QChar ch) { if (size <= 0) { - d = const_cast(&shared_empty.str); + d = shared_empty.data_ptr(); } else { d = (Data*) ::malloc(sizeof(Data)+(size+1)*sizeof(QChar)); Q_CHECK_PTR(d); @@ -1240,7 +1240,7 @@ void QString::resize(int size) } if (size == 0 && !d->capacityReserved) { - Data *x = const_cast(&shared_empty.str); + Data *x = shared_empty.data_ptr(); if (!d->ref.deref()) QString::free(d); d = x; @@ -4064,9 +4064,9 @@ QString::Data *QString::fromLatin1_helper(const char *str, int size) { Data *d; if (!str) { - d = const_cast(&shared_null.str); + d = shared_null.data_ptr(); } else if (size == 0 || (!*str && size < 0)) { - d = const_cast(&shared_empty.str); + d = shared_empty.data_ptr(); } else { if (size < 0) size = qstrlen(str); @@ -4141,8 +4141,10 @@ QString QString::fromLocal8Bit_helper(const char *str, int size) { if (!str) return QString(); - if (size == 0 || (!*str && size < 0)) - return QString(shared_empty); + if (size == 0 || (!*str && size < 0)) { + QStringDataPtr empty = { shared_empty.data_ptr() }; + return QString(empty); + } #if !defined(QT_NO_TEXTCODEC) if (size < 0) size = qstrlen(str); @@ -4309,7 +4311,8 @@ QString QString::simplified() const break; if (++from == fromEnd) { // All-whitespace string - return QString(shared_empty); + QStringDataPtr empty = { shared_empty.data_ptr() }; + return QString(empty); } } // This loop needs no underflow check, as we already determined that @@ -4403,7 +4406,8 @@ QString QString::trimmed() const } int l = end - start + 1; if (l <= 0) { - return QString(shared_empty); + QStringDataPtr empty = { shared_empty.data_ptr() }; + return QString(empty); } return QString(s + start, l); } @@ -7436,9 +7440,9 @@ QString QString::fromRawData(const QChar *unicode, int size) { Data *x; if (!unicode) { - x = const_cast(&shared_null.str); + x = shared_null.data_ptr(); } else if (!size) { - x = const_cast(&shared_empty.str); + x = shared_empty.data_ptr(); } else { x = static_cast(::malloc(sizeof(Data) + sizeof(ushort))); Q_CHECK_PTR(x); @@ -7448,7 +7452,8 @@ QString QString::fromRawData(const QChar *unicode, int size) x->capacityReserved = false; x->offset = reinterpret_cast(unicode) - reinterpret_cast(x); } - return QString(x, 0); + QStringDataPtr dataPtr = { x }; + return QString(dataPtr); } /*! diff --git a/src/corelib/tools/qstring.h b/src/corelib/tools/qstring.h index 4f241e72e2..50796a9e29 100644 --- a/src/corelib/tools/qstring.h +++ b/src/corelib/tools/qstring.h @@ -113,12 +113,16 @@ Q_STATIC_ASSERT_X(sizeof(qunicodechar) == 2, #ifndef QT_NO_UNICODE_LITERAL # define QT_UNICODE_LITERAL(str) QT_UNICODE_LITERAL_II(str) # if defined(Q_COMPILER_LAMBDA) -# define QStringLiteral(str) ([]() -> QStaticStringDataPtr { \ + +# define QStringLiteral(str) \ + ([]() -> QStringDataPtr { \ enum { Size = sizeof(QT_UNICODE_LITERAL(str))/2 - 1 }; \ static const QStaticStringData qstring_literal = \ { { Q_REFCOUNT_INITIALIZE_STATIC, Size, 0, 0, sizeof(QStringData) }, QT_UNICODE_LITERAL(str) }; \ - QStaticStringDataPtr holder = { &qstring_literal }; \ - return holder; }()) + QStringDataPtr holder = { qstring_literal.data_ptr() }; \ + return holder; \ + }()) \ + /**/ # elif defined(Q_CC_GNU) // We need to create a QStringData in the .rodata section of memory @@ -130,8 +134,11 @@ Q_STATIC_ASSERT_X(sizeof(qunicodechar) == 2, enum { Size = sizeof(QT_UNICODE_LITERAL(str))/2 - 1 }; \ static const QStaticStringData qstring_literal = \ { { Q_REFCOUNT_INITIALIZE_STATIC, Size, 0, 0, sizeof(QStringData) }, QT_UNICODE_LITERAL(str) }; \ - QStaticStringDataPtr holder = { &qstring_literal }; \ - holder; }) + QStringDataPtr holder = { qstring_literal.data_ptr() }; \ + holder; \ + }) \ + /**/ + # endif #endif // QT_NO_UNICODE_LITERAL @@ -147,12 +154,17 @@ struct QStaticStringData { QStringData str; qunicodechar data[N + 1]; + + QStringData *data_ptr() const + { + Q_ASSERT(str.ref.isStatic()); + return const_cast(&str); + } }; -template -struct QStaticStringDataPtr +struct QStringDataPtr { - const QStaticStringData *ptr; + QStringData *ptr; }; class Q_CORE_EXPORT QString @@ -421,11 +433,13 @@ public: // note - this are all inline so we can benefit from strlen() compile time optimizations static inline QString fromAscii(const char *str, int size = -1) { - return QString(fromAscii_helper(str, (str && size == -1) ? int(strlen(str)) : size), 0); + QStringDataPtr dataPtr = { fromAscii_helper(str, (str && size == -1) ? int(strlen(str)) : size) }; + return QString(dataPtr); } static inline QString fromLatin1(const char *str, int size = -1) { - return QString(fromLatin1_helper(str, (str && size == -1) ? int(strlen(str)) : size), 0); + QStringDataPtr dataPtr = { fromLatin1_helper(str, (str && size == -1) ? int(strlen(str)) : size) }; + return QString(dataPtr); } static inline QString fromUtf8(const char *str, int size = -1) { @@ -600,7 +614,7 @@ public: // compatibility struct Null { }; static const Null null; - inline QString(const Null &): d(const_cast(&shared_null.str)) {} + inline QString(const Null &): d(shared_null.data_ptr()) {} inline QString &operator=(const Null &) { *this = QString(); return *this; } inline bool isNull() const { return d == &shared_null.str; } @@ -609,10 +623,7 @@ public: bool isRightToLeft() const; QString(int size, Qt::Initialization); - template - inline QString(const QStaticStringData &dd) : d(const_cast(&dd.str)) {} - template - Q_DECL_CONSTEXPR inline QString(QStaticStringDataPtr dd) : d(const_cast(&dd.ptr->str)) {} + Q_DECL_CONSTEXPR inline QString(QStringDataPtr dd) : d(dd.ptr) {} private: #if defined(QT_NO_CAST_FROM_ASCII) && !defined(Q_NO_DECLARED_NOT_DEFINED) @@ -627,7 +638,6 @@ private: static const QStaticStringData<1> shared_null; static const QStaticStringData<1> shared_empty; Data *d; - inline QString(Data *dd, int /*dummy*/) : d(dd) {} static int grow(int); static void free(Data *); @@ -888,7 +898,7 @@ inline void QCharRef::setRow(uchar arow) { QChar(*this).setRow(arow); } inline void QCharRef::setCell(uchar acell) { QChar(*this).setCell(acell); } -inline QString::QString() : d(const_cast(&shared_null.str)) {} +inline QString::QString() : d(shared_null.data_ptr()) {} inline QString::~QString() { if (!d->ref.deref()) free(d); } inline void QString::reserve(int asize) diff --git a/src/corelib/tools/qstringbuilder.h b/src/corelib/tools/qstringbuilder.h index 0b85e590cd..9a1fd6949b 100644 --- a/src/corelib/tools/qstringbuilder.h +++ b/src/corelib/tools/qstringbuilder.h @@ -253,16 +253,16 @@ template <> struct QConcatenable : private QAbstractConcatenable } }; -template struct QConcatenable > : private QAbstractConcatenable +template <> struct QConcatenable : private QAbstractConcatenable { - typedef QStaticStringDataPtr type; + typedef QStringDataPtr type; typedef QString ConvertTo; enum { ExactSize = true }; - static int size(const type &) { return N; } + static int size(const type &a) { return a.ptr->size; } static inline void appendTo(const type &a, QChar *&out) { - memcpy(out, reinterpret_cast(a.ptr->data), sizeof(QChar) * N); - out += N; + memcpy(out, reinterpret_cast(a.ptr->data()), sizeof(QChar) * a.ptr->size); + out += a.ptr->size; } }; @@ -358,22 +358,22 @@ template <> struct QConcatenable : private QAbstractConcatenable } }; -template struct QConcatenable > : private QAbstractConcatenable +template <> struct QConcatenable : private QAbstractConcatenable { - typedef QStaticByteArrayDataPtr type; + typedef QByteArrayDataPtr type; typedef QByteArray ConvertTo; enum { ExactSize = false }; - static int size(const type &) { return N; } + static int size(const type &ba) { return ba.ptr->size; } #ifndef QT_NO_CAST_FROM_ASCII static inline QT_ASCII_CAST_WARN void appendTo(const type &a, QChar *&out) { - QAbstractConcatenable::convertFromAscii(a.ptr->data, N, out); + QAbstractConcatenable::convertFromAscii(a.ptr->data(), a.ptr->size, out); } #endif static inline void appendTo(const type &ba, char *&out) { - ::memcpy(out, ba.ptr->data, N); - out += N; + ::memcpy(out, ba.ptr->data(), ba.ptr->size); + out += ba.ptr->size; } }; diff --git a/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp b/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp index fe9b5f67c4..72493d3956 100644 --- a/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp +++ b/tests/auto/corelib/tools/qbytearray/tst_qbytearray.cpp @@ -181,7 +181,7 @@ private slots: } while (false) \ /**/ -struct StaticByteArrays { +static const struct StaticByteArrays { struct Standard { QByteArrayData data; const char string[8]; @@ -207,10 +207,10 @@ struct StaticByteArrays { ,{{ Q_REFCOUNT_INITIALIZE_STATIC, /* length = */ 4, 0, 0, sizeof(QByteArrayData) + sizeof(char) }, 0, "dataBAD"} }; -static const QStaticByteArrayData<1> &staticStandard = reinterpret_cast &>(statics.standard); -static const QStaticByteArrayData<1> &staticNotNullTerminated = reinterpret_cast &>(statics.notNullTerminated); -static const QStaticByteArrayData<1> &staticShifted = reinterpret_cast &>(statics.shifted); -static const QStaticByteArrayData<1> &staticShiftedNotNullTerminated = reinterpret_cast &>(statics.shiftedNotNullTerminated); +static const QByteArrayDataPtr staticStandard = { const_cast(&statics.standard.data) }; +static const QByteArrayDataPtr staticNotNullTerminated = { const_cast(&statics.notNullTerminated.data) }; +static const QByteArrayDataPtr staticShifted = { const_cast(&statics.shifted.data) }; +static const QByteArrayDataPtr staticShiftedNotNullTerminated = { const_cast(&statics.shiftedNotNullTerminated.data) }; tst_QByteArray::tst_QByteArray() { diff --git a/tests/auto/corelib/tools/qstringbuilder/qstringbuilder1/stringbuilder.cpp b/tests/auto/corelib/tools/qstringbuilder/qstringbuilder1/stringbuilder.cpp index 556b9ac16a..429652d92d 100644 --- a/tests/auto/corelib/tools/qstringbuilder/qstringbuilder1/stringbuilder.cpp +++ b/tests/auto/corelib/tools/qstringbuilder/qstringbuilder1/stringbuilder.cpp @@ -107,6 +107,21 @@ void runScenario() QCOMPARE(r, r3); #endif + { + static const QStaticStringData<12> literalData = { + { Q_REFCOUNT_INITIALIZE_STATIC, 12, 0, 0, sizeof(QStringData) }, + { 's', 'o', 'm', 'e', ' ', 'l', 'i', 't', 'e', 'r', 'a', 'l' } + }; + static QStringDataPtr literal = { literalData.data_ptr() }; + + r = literal; + QCOMPARE(r, string); + r = r Q literal; + QCOMPARE(r, r2); + r = literal Q literal; + QCOMPARE(r, r2); + } + #ifndef QT_NO_CAST_FROM_ASCII r = string P LITERAL; QCOMPARE(r, r2); @@ -211,6 +226,21 @@ void runScenario() QCOMPARE(r, ba); } + { + static const QStaticByteArrayData<12> literalData = { + { Q_REFCOUNT_INITIALIZE_STATIC, 12, 0, 0, sizeof(QByteArrayData) }, + { 's', 'o', 'm', 'e', ' ', 'l', 'i', 't', 'e', 'r', 'a', 'l' } + }; + static QByteArrayDataPtr literal = { literalData.data_ptr() }; + + QByteArray ba = literal; + QCOMPARE(ba, QByteArray(LITERAL)); + ba = ba Q literal; + QCOMPARE(ba, QByteArray(LITERAL LITERAL)); + ba = literal Q literal; + QCOMPARE(ba, QByteArray(LITERAL LITERAL)); + } + //operator QString += { QString str = QString::fromUtf8(UTF8_LITERAL); -- cgit v1.2.3