From d76c5fb7feda66621b435263353f49e9b4b34308 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 14 May 2009 18:30:57 +0200 Subject: Fix handling of dynamic casts in QSharedPointer. It's wrong to assume that static_cast<> is allowed everywhere where dynamic_cast<> is allowed. For example, if class C derives from both A and B, then you can dynamic_cast(ptr_to_A), but you can't static_cast. So introduce a helper for dynamic casts that doesn't do static_cast. Reviewed-by: Olivier Goffart --- src/corelib/tools/qsharedpointer_impl.h | 21 ++-- tests/auto/qsharedpointer/tst_qsharedpointer.cpp | 137 +++++++++++++++++++++-- 2 files changed, 140 insertions(+), 18 deletions(-) diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index f1b35ee..ad2d9f2 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -238,6 +238,7 @@ namespace QtSharedPointer { template friend class ExternalRefCount; template friend class QWeakPointer; template friend QSharedPointer qSharedPointerCastHelper(const QSharedPointer &src, X *); + template friend QSharedPointer qSharedPointerDynamicCastHelper(const QSharedPointer &src, X *); template friend QSharedPointer qSharedPointerConstCastHelper(const QSharedPointer &src, X *); template friend QSharedPointer QtSharedPointer::qStrongRefFromWeakHelper(const QWeakPointer &src, X *); #endif @@ -509,6 +510,14 @@ namespace QtSharedPointer { return result; } template + Q_INLINE_TEMPLATE QSharedPointer qSharedPointerDynamicCastHelper(const QSharedPointer &src, X *) + { + QSharedPointer result; + register T *ptr = src.data(); + result.internalSet(src.d, dynamic_cast(ptr)); + return result; + } + template Q_INLINE_TEMPLATE QSharedPointer qSharedPointerConstCastHelper(const QSharedPointer &src, X *) { QSharedPointer result; @@ -544,9 +553,7 @@ template Q_INLINE_TEMPLATE QSharedPointer qSharedPointerDynamicCast(const QSharedPointer &src) { X *x = 0; - if (QtSharedPointer::qVerifyDynamicCast(src.data(), x)) - return QtSharedPointer::qSharedPointerCastHelper(src, x); - return QSharedPointer(); + return QtSharedPointer::qSharedPointerDynamicCastHelper(src, x); } template Q_INLINE_TEMPLATE QSharedPointer qSharedPointerDynamicCast(const QWeakPointer &src) @@ -558,17 +565,13 @@ template Q_INLINE_TEMPLATE QSharedPointer qSharedPointerConstCast(const QSharedPointer &src) { X *x = 0; - if (QtSharedPointer::qVerifyConstCast(src.data(), x)) - return QtSharedPointer::qSharedPointerConstCastHelper(src, x); - return QSharedPointer(); + return QtSharedPointer::qSharedPointerConstCastHelper(src, x); } template Q_INLINE_TEMPLATE QSharedPointer qSharedPointerConstCast(const QWeakPointer &src) { X *x = 0; - if (QtSharedPointer::qVerifyConstCast(src.data(), x)) - return QtSharedPointer::qSharedPointerCastHelper(src, x); - return QSharedPointer(); + return QtSharedPointer::qSharedPointerConstCastHelper(src, x); } template diff --git a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp index 64439fb..a52bb3e 100644 --- a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp @@ -57,8 +57,11 @@ private slots: void downCast(); void upCast(); void differentPointers(); + void virtualBaseDifferentPointers(); #ifndef QTEST_NO_RTTI void dynamicCast(); + void dynamicCastDifferentPointers(); + void dynamicCastVirtualBase(); void dynamicCastFailure(); #endif void customDeleter(); @@ -321,6 +324,10 @@ class DiffPtrDerivedData: public Stuffing, public Data { }; +class VirtualDerived: virtual public Data +{ +}; + void tst_QSharedPointer::downCast() { { @@ -439,7 +446,7 @@ void tst_QSharedPointer::differentPointers() DiffPtrDerivedData *aData = new DiffPtrDerivedData; Data *aBase = aData; Q_ASSERT(aData == aBase); - Q_ASSERT(quintptr(&aData) != quintptr(&aBase)); + Q_ASSERT(*reinterpret_cast(&aData) != *reinterpret_cast(&aBase)); QSharedPointer baseptr = QSharedPointer(aData); QSharedPointer ptr = qSharedPointerCast(baseptr); @@ -453,7 +460,7 @@ void tst_QSharedPointer::differentPointers() DiffPtrDerivedData *aData = new DiffPtrDerivedData; Data *aBase = aData; Q_ASSERT(aData == aBase); - Q_ASSERT(quintptr(&aData) != quintptr(&aBase)); + Q_ASSERT(*reinterpret_cast(&aData) != *reinterpret_cast(&aBase)); QSharedPointer ptr = QSharedPointer(aData); QSharedPointer baseptr = ptr; @@ -464,23 +471,53 @@ void tst_QSharedPointer::differentPointers() QVERIFY(baseptr == aData); QVERIFY(baseptr == aBase); } +} + +void tst_QSharedPointer::virtualBaseDifferentPointers() +{ + { + VirtualDerived *aData = new VirtualDerived; + Data *aBase = aData; + Q_ASSERT(aData == aBase); + Q_ASSERT(*reinterpret_cast(&aData) != *reinterpret_cast(&aBase)); + + QSharedPointer ptr = QSharedPointer(aData); + QSharedPointer baseptr = qSharedPointerCast(ptr); + QVERIFY(ptr == baseptr); + QVERIFY(ptr.data() == baseptr.data()); + QVERIFY(ptr == aBase); + QVERIFY(ptr == aData); + QVERIFY(baseptr == aData); + QVERIFY(baseptr == aBase); + } + + { + VirtualDerived *aData = new VirtualDerived; + Data *aBase = aData; + Q_ASSERT(aData == aBase); + Q_ASSERT(*reinterpret_cast(&aData) != *reinterpret_cast(&aBase)); - // there is no possibility for different pointers in - // internal reference counting right now - // - // to do that, it's necessary to first implement the ability to - // call (virtual) functions, so that the two differing bases have - // the same reference counter + QSharedPointer ptr = QSharedPointer(aData); + QSharedPointer baseptr = ptr; + QVERIFY(ptr == baseptr); + QVERIFY(ptr.data() == baseptr.data()); + QVERIFY(ptr == aBase); + QVERIFY(ptr == aData); + QVERIFY(baseptr == aData); + QVERIFY(baseptr == aBase); + } } #ifndef QTEST_NO_RTTI void tst_QSharedPointer::dynamicCast() { - QSharedPointer baseptr = QSharedPointer(new DerivedData); + DerivedData *aData = new DerivedData; + QSharedPointer baseptr = QSharedPointer(aData); { QSharedPointer derivedptr = qSharedPointerDynamicCast(baseptr); QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); } QCOMPARE(int(baseptr.d->weakref), 1); @@ -490,6 +527,7 @@ void tst_QSharedPointer::dynamicCast() QWeakPointer weakptr = baseptr; QSharedPointer derivedptr = qSharedPointerDynamicCast(weakptr); QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); } QCOMPARE(int(baseptr.d->weakref), 1); @@ -498,6 +536,87 @@ void tst_QSharedPointer::dynamicCast() { QSharedPointer derivedptr = baseptr.dynamicCast(); QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); + QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); + } + QCOMPARE(int(baseptr.d->weakref), 1); + QCOMPARE(int(baseptr.d->strongref), 1); +} + +void tst_QSharedPointer::dynamicCastDifferentPointers() +{ + // DiffPtrDerivedData derives from both Data and Stuffing + DiffPtrDerivedData *aData = new DiffPtrDerivedData; + QSharedPointer baseptr = QSharedPointer(aData); + + { + QSharedPointer derivedptr = qSharedPointerDynamicCast(baseptr); + QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); + QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); + } + QCOMPARE(int(baseptr.d->weakref), 1); + QCOMPARE(int(baseptr.d->strongref), 1); + + { + QWeakPointer weakptr = baseptr; + QSharedPointer derivedptr = qSharedPointerDynamicCast(weakptr); + QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); + QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); + } + QCOMPARE(int(baseptr.d->weakref), 1); + QCOMPARE(int(baseptr.d->strongref), 1); + + { + QSharedPointer derivedptr = baseptr.dynamicCast(); + QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); + QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); + } + QCOMPARE(int(baseptr.d->weakref), 1); + QCOMPARE(int(baseptr.d->strongref), 1); + + { + Stuffing *nakedptr = dynamic_cast(baseptr.data()); + QVERIFY(nakedptr); + + QSharedPointer otherbaseptr = qSharedPointerDynamicCast(baseptr); + QVERIFY(!otherbaseptr.isNull()); + QVERIFY(otherbaseptr == nakedptr); + QCOMPARE(otherbaseptr.data(), nakedptr); + QCOMPARE(static_cast(otherbaseptr.data()), aData); + } +} + +void tst_QSharedPointer::dynamicCastVirtualBase() +{ + VirtualDerived *aData = new VirtualDerived; + QSharedPointer baseptr = QSharedPointer(aData); + + { + QSharedPointer derivedptr = qSharedPointerDynamicCast(baseptr); + QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); + QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); + } + QCOMPARE(int(baseptr.d->weakref), 1); + QCOMPARE(int(baseptr.d->strongref), 1); + + { + QWeakPointer weakptr = baseptr; + QSharedPointer derivedptr = qSharedPointerDynamicCast(weakptr); + QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); + QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); + } + QCOMPARE(int(baseptr.d->weakref), 1); + QCOMPARE(int(baseptr.d->strongref), 1); + + { + QSharedPointer derivedptr = baseptr.dynamicCast(); + QVERIFY(baseptr == derivedptr); + QCOMPARE(derivedptr.data(), aData); QCOMPARE(static_cast(derivedptr.data()), baseptr.data()); } QCOMPARE(int(baseptr.d->weakref), 1); -- cgit v0.12