From b9fd0b1272efe83ad2c959b80eeed816207c0c28 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 27 Mar 2025 09:45:26 +0100 Subject: [PATCH] tst_QSplitter: fix memleak in replaceWidget() Depending on whether QSplitter::replaceWidget() succeeds or not, we need to delete either the return value (an unparented QWidget) or the argument (another QWidget without parent). The test function only deleted the return value. Use a unique_ptr for delayed automatic deletion at the end of the test function, which is the minimally-invasive fix. Use it for both candidates, which ensures that either one of them is reliably deleted, even if a QCOMPARE() fails in-between. One check before we reset(newWidget) is special: if (!res) fails, then the reaper contained something we also need to delete, and the reset() will do that, but code checkers (e.g. Coverity) are known to complain about using a pointer value after a delete, so split the check into the check and the QVERIFY(). Amends 2c634a132696376a0a0d7641bf05ae875c50190e. Pick-to: 6.8 6.5 5.15 Change-Id: I8571477d9e53b85b2db041d7c9bdf2bd7aa69906 Reviewed-by: Axel Spoerl (cherry picked from commit 4513d48490f39c805644ea01a5a8e9a16d51764b) Reviewed-by: Qt Cherry-pick Bot --- tests/auto/widgets/widgets/qsplitter/tst_qsplitter.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/auto/widgets/widgets/qsplitter/tst_qsplitter.cpp b/tests/auto/widgets/widgets/qsplitter/tst_qsplitter.cpp index 465df2c1f49..5133b22a754 100644 --- a/tests/auto/widgets/widgets/qsplitter/tst_qsplitter.cpp +++ b/tests/auto/widgets/widgets/qsplitter/tst_qsplitter.cpp @@ -748,11 +748,15 @@ void tst_QSplitter::replaceWidget() EventCounterSpy ef(&sp); ef.installEventFilter(); const QWidget *res = sp.replaceWidget(index, newWidget); + std::unique_ptr reaper{res}; QTest::qWait(100); // Give visibility and resizing some time // Check if (index < 0 || index >= count) { - QVERIFY(!res); + // check before reset() may delete *res: C++/Coverity are picky with zapped pointers + const bool resWasNull = !res; + reaper.reset(newWidget); // newWidget wasn't added, so we need to delete + QVERIFY(resWasNull); QVERIFY(!newWidget->parentWidget()); QCOMPARE(ef.resizeCount, 0); QCOMPARE(ef.paintCount, 0); @@ -769,7 +773,6 @@ void tst_QSplitter::replaceWidget() if (visible && !collapsed) QCOMPARE(newWidget->geometry(), oldGeom); QCOMPARE(newWidget->size().isEmpty(), !visible || collapsed); - delete res; } QCOMPARE(sp.count(), count); QCOMPARE(sp.sizes(), sizes);