From 88974b0a3ef7bc02b4f62f7ce1ded3efaf003ec8 Mon Sep 17 00:00:00 2001
From: Ivan Solovev <ivan.solovev@qt.io>
Date: Wed, 17 Aug 2022 16:01:03 +0200
Subject: [PATCH] tst_qscopedpointer: port away from deprecated APIs

The QScopedPointer::take() call in comparison() test was used to
avoid a double-deletion error, because the test is creating two
QScopedPointer instances referencing the same memory.

Avoid the take() call by providing a custom DummyDeleter and
managing the memory by the extarnal std::unique_ptr.

As the test now has no test-cases for QScopedPointer::take()
calls, create a new test for this deprecated API, and guard
it with QT_DEPRECATED_SINCE checks.

Task-number: QTBUG-104858
Change-Id: Iecc28d44d76c9ce5835e6b1a1df7db30e2a9ca25
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 .../qscopedpointer/tst_qscopedpointer.cpp     | 68 +++++++++++++------
 1 file changed, 48 insertions(+), 20 deletions(-)

diff --git a/tests/auto/corelib/tools/qscopedpointer/tst_qscopedpointer.cpp b/tests/auto/corelib/tools/qscopedpointer/tst_qscopedpointer.cpp
index ed0a38a5e1b..367cfb5b5d7 100644
--- a/tests/auto/corelib/tools/qscopedpointer/tst_qscopedpointer.cpp
+++ b/tests/auto/corelib/tools/qscopedpointer/tst_qscopedpointer.cpp
@@ -36,6 +36,11 @@ private Q_SLOTS:
     void comparison();
     void array();
     // TODO instanciate on const object
+
+    // Tests for deprecated APIs
+#if QT_DEPRECATED_SINCE(6, 1)
+    void deprecatedTake();
+#endif // QT_DEPRECATED_SINCE(6, 1)
 };
 
 void tst_QScopedPointer::defaultConstructor()
@@ -342,51 +347,57 @@ void scopedPointerComparisonTest(const A1 &a1, const A2 &a2, const B &b)
     QVERIFY(a2 != b);
 }
 
+// tst_QScopedPointer::comparison creates two QScopedPointers referring to the
+// same memory. This will lead to double-deletion error during cleanup if we
+// use a default QScopedPointer{Array}Deleter. This DummyDeleter does nothing,
+// so we can safely reference the same memory from multiple QScopedPointer
+// instances, and manage the memory manually.
+// That is fine for the comparison() test, because its goal is to check the
+// object's (in)equality, not the memory management
+struct DummyDeleter
+{
+    static inline void cleanup(RefCounted *) noexcept {}
+    void operator()(RefCounted *pointer) const noexcept
+    {
+        cleanup(pointer);
+    }
+};
+
 void tst_QScopedPointer::comparison()
 {
     QCOMPARE( RefCounted::instanceCount.loadRelaxed(), 0 );
 
     {
-        RefCounted *a = new RefCounted;
-        RefCounted *b = new RefCounted;
+        auto a = std::make_unique<RefCounted>();
+        auto b = std::make_unique<RefCounted>();
 
         QCOMPARE( RefCounted::instanceCount.loadRelaxed(), 2 );
 
-        QScopedPointer<RefCounted> pa1(a);
-        QScopedPointer<RefCounted> pa2(a);
-        QScopedPointer<RefCounted> pb(b);
+        QScopedPointer<RefCounted, DummyDeleter> pa1(a.get());
+        QScopedPointer<RefCounted, DummyDeleter> pa2(a.get());
+        QScopedPointer<RefCounted, DummyDeleter> pb(b.get());
 
         scopedPointerComparisonTest(pa1, pa1, pb);
         scopedPointerComparisonTest(pa2, pa2, pb);
         scopedPointerComparisonTest(pa1, pa2, pb);
 
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-        pa2.take();
-QT_WARNING_POP
-
         QCOMPARE( RefCounted::instanceCount.loadRelaxed(), 2 );
     }
 
     QCOMPARE( RefCounted::instanceCount.loadRelaxed(), 0 );
 
     {
-        RefCounted *a = new RefCounted[42];
-        RefCounted *b = new RefCounted[43];
+        auto a = std::make_unique<RefCounted[]>(42);
+        auto b = std::make_unique<RefCounted[]>(43);
 
         QCOMPARE( RefCounted::instanceCount.loadRelaxed(), 85 );
 
-        QScopedArrayPointer<RefCounted> pa1(a);
-        QScopedArrayPointer<RefCounted> pa2(a);
-        QScopedArrayPointer<RefCounted> pb(b);
+        QScopedArrayPointer<RefCounted, DummyDeleter> pa1(a.get());
+        QScopedArrayPointer<RefCounted, DummyDeleter> pa2(a.get());
+        QScopedArrayPointer<RefCounted, DummyDeleter> pb(b.get());
 
         scopedPointerComparisonTest(pa1, pa2, pb);
 
-QT_WARNING_PUSH
-QT_WARNING_DISABLE_DEPRECATED
-        pa2.take();
-QT_WARNING_POP
-
         QCOMPARE( RefCounted::instanceCount.loadRelaxed(), 85 );
     }
 
@@ -434,6 +445,23 @@ void tst_QScopedPointer::array()
     QCOMPARE(instCount, RefCounted::instanceCount.loadRelaxed());
 }
 
+#if QT_DEPRECATED_SINCE(6, 1)
+void tst_QScopedPointer::deprecatedTake()
+{
+    RefCounted *a = new RefCounted;
+
+    QScopedPointer<RefCounted> pa1(a);
+    QScopedPointer<RefCounted> pa2(a);
+
+    QCOMPARE(RefCounted::instanceCount.loadRelaxed(), 1);
+
+    QT_IGNORE_DEPRECATIONS(pa2.take();)
+
+    // check that pa2 holds nullptr, but the memory was not released
+    QVERIFY(pa2.isNull());
+    QCOMPARE(RefCounted::instanceCount.loadRelaxed(), 1);
+}
+#endif // QT_DEPRECATED_SINCE(6, 1)
 
 QTEST_MAIN(tst_QScopedPointer)
 #include "tst_qscopedpointer.moc"