From 20eba275d836e071c1ad8a5e4d1ef88fc5b23fca Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Tue, 19 Mar 2024 18:31:28 +0100
Subject: [PATCH] tst_QNetworkReply: don't leak on failure in
 emitErrorForAllReplies()

The old code allocated QSignalSpies on the heap and stored them in a
non-owning container, so if one of the many check macros trigger,
those objects would be leaked. Ditto QNetworkReplies. The code also
used dynamically-sized containers for statically-sized data; a common
anti-pattern.

Hold the sample QUrls in a C array instead, QSignalSpies in C arrays
of std::optional (to delay initialization) and QNetworkReplies in a C
array of std::unique_ptr with the existing QScopedPointerDeleteLater
deleter.

Pick-to: 6.7
Change-Id: I7305115af15c079abba6d45c5de8db2198ea7a6d
Reviewed-by: Juha Vuolle <juha.vuolle@qt.io>
---
 .../qnetworkreply/tst_qnetworkreply.cpp       | 42 ++++++++++---------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
index 243ad60c09..50567f4e43 100644
--- a/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
+++ b/tests/auto/network/access/qnetworkreply/tst_qnetworkreply.cpp
@@ -8808,31 +8808,33 @@ void tst_QNetworkReply::ftpAuthentication()
 void tst_QNetworkReply::emitErrorForAllReplies() // QTBUG-36890
 {
     // port 100 is not well-known and should be closed
-    QList<QUrl> urls = QList<QUrl>() << QUrl("http://localhost:100/request1")
-                                     << QUrl("http://localhost:100/request2")
-                                     << QUrl("http://localhost:100/request3");
-    QList<QNetworkReply *> replies;
-    QList<QSignalSpy *> errorSpies;
-    QList<QSignalSpy *> finishedSpies;
-    for (int a = 0; a < urls.size(); ++a) {
-        QNetworkRequest request(urls.at(a));
+    const QUrl urls[] = {
+        QUrl("http://localhost:100/request1"),
+        QUrl("http://localhost:100/request2"),
+        QUrl("http://localhost:100/request3"),
+    };
+    constexpr auto NUrls = std::size(urls);
+
+    std::unique_ptr<QNetworkReply, QScopedPointerDeleteLater> replies[NUrls];
+    std::optional<QSignalSpy> errorSpies[NUrls];
+    std::optional<QSignalSpy> finishedSpies[NUrls];
+
+    for (size_t i = 0; i < NUrls; ++i) {
+        QNetworkRequest request(urls[i]);
         QNetworkReply *reply = manager.get(request);
-        replies.append(reply);
-        QSignalSpy *errorSpy = new QSignalSpy(reply, SIGNAL(errorOccurred(QNetworkReply::NetworkError)));
-        errorSpies.append(errorSpy);
-        QSignalSpy *finishedSpy = new QSignalSpy(reply, SIGNAL(finished()));
-        finishedSpies.append(finishedSpy);
+        replies[i].reset(reply);
+        errorSpies[i].emplace(reply, SIGNAL(errorOccurred(QNetworkReply::NetworkError)));
+        finishedSpies[i].emplace(reply, SIGNAL(finished()));
         QObject::connect(reply, SIGNAL(finished()), SLOT(emitErrorForAllRepliesSlot()));
     }
+
     QTestEventLoop::instance().enterLoop(10);
     QVERIFY(!QTestEventLoop::instance().timeout());
-    for (int a = 0; a < urls.size(); ++a) {
-        QVERIFY(replies.at(a)->isFinished());
-        QCOMPARE(errorSpies.at(a)->size(), 1);
-        delete errorSpies.at(a);
-        QCOMPARE(finishedSpies.at(a)->size(), 1);
-        delete finishedSpies.at(a);
-        replies.at(a)->deleteLater();
+
+    for (size_t i = 0; i < NUrls; ++i) {
+        QVERIFY(replies[i]->isFinished());
+        QCOMPARE(errorSpies[i]->size(), 1);
+        QCOMPARE(finishedSpies[i]->size(), 1);
     }
 }