From e70e168abb4b75dc8d00e2cba12efa1a111d429d Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 29 Sep 2016 09:06:12 +0200 Subject: Plug leaks in tst_QWizard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This completely over-engineered piece of code has a hierarchy of Operation subclasses encapsulating but three actual operations on a QWizard. Because these operations and their containers were all allocated on the heap, but never deleted, asan went crazy and reported over 50 leaks (not the record so far, but a (distant) second). Since these collections are passed through addColumn/QFETCH, too, it's nearly impossible to track their lifetimes. So instead of trying, delegate that to the runtime, ie. pack the Operation objects into QSharedPointer and pass around those instead. Change-Id: I8a0fe7a60cd30aed618667affaa030e80cf2b1ac Reviewed-by: Edward Welbourne Reviewed-by: Sérgio Martins --- tests/auto/widgets/dialogs/qwizard/tst_qwizard.cpp | 106 ++++++++++++--------- 1 file changed, 60 insertions(+), 46 deletions(-) (limited to 'tests') diff --git a/tests/auto/widgets/dialogs/qwizard/tst_qwizard.cpp b/tests/auto/widgets/dialogs/qwizard/tst_qwizard.cpp index b2bdbac79a..5789f0ca42 100644 --- a/tests/auto/widgets/dialogs/qwizard/tst_qwizard.cpp +++ b/tests/auto/widgets/dialogs/qwizard/tst_qwizard.cpp @@ -1626,28 +1626,44 @@ class SetPage : public Operation wizard->next(); } QString describe() const { return QString("set page %1").arg(page); } - const int page; + int page; public: - SetPage(int page) : page(page) {} + static QSharedPointer create(int page) + { + QSharedPointer o = QSharedPointer::create(); + o->page = page; + return o; + } }; class SetStyle : public Operation { void apply(QWizard *wizard) const { wizard->setWizardStyle(style); } QString describe() const { return QString("set style %1").arg(style); } - const QWizard::WizardStyle style; + QWizard::WizardStyle style; public: - SetStyle(QWizard::WizardStyle style) : style(style) {} + static QSharedPointer create(QWizard::WizardStyle style) + { + QSharedPointer o = QSharedPointer::create(); + o->style = style; + return o; + } }; class SetOption : public Operation { void apply(QWizard *wizard) const { wizard->setOption(option, on); } QString describe() const; - const QWizard::WizardOption option; - const bool on; + QWizard::WizardOption option; + bool on; public: - SetOption(QWizard::WizardOption option, bool on) : option(option), on(on) {} + static QSharedPointer create(QWizard::WizardOption option, bool on) + { + QSharedPointer o = QSharedPointer::create(); + o->option = option; + o->on = on; + return o; + } }; class OptionInfo @@ -1672,16 +1688,16 @@ class OptionInfo tags[QWizard::HaveCustomButton3] = "15/CB3"; for (int i = 0; i < 2; ++i) { - QMap operations_; + QMap > operations_; foreach (QWizard::WizardOption option, tags.keys()) - operations_[option] = new SetOption(option, i == 1); + operations_[option] = SetOption::create(option, i == 1); operations << operations_; } } OptionInfo(OptionInfo const&); OptionInfo& operator=(OptionInfo const&); QMap tags; - QList > operations; + QList > > operations; public: static OptionInfo &instance() { @@ -1690,7 +1706,7 @@ public: } QString tag(QWizard::WizardOption option) const { return tags.value(option); } - Operation * operation(QWizard::WizardOption option, bool on) const + QSharedPointer operation(QWizard::WizardOption option, bool on) const { return operations.at(on).value(option); } QList options() const { return tags.keys(); } }; @@ -1700,10 +1716,7 @@ QString SetOption::describe() const return QString("set opt %1 %2").arg(OptionInfo::instance().tag(option)).arg(on); } -Q_DECLARE_METATYPE(Operation *) -Q_DECLARE_METATYPE(SetPage *) -Q_DECLARE_METATYPE(SetStyle *) -Q_DECLARE_METATYPE(SetOption *) +Q_DECLARE_METATYPE(QVector >) class TestGroup { @@ -1720,14 +1733,17 @@ public: combinations.clear(); } - QList &add() - { combinations << new QList; return *(combinations.last()); } + QVector > &add() + { + combinations.resize(combinations.size() + 1); + return combinations.last(); + } void createTestRows() { for (int i = 0; i < combinations.count(); ++i) { QTest::newRow((name + QString(", row %1").arg(i)).toLatin1().data()) - << (i == 0) << (type == Equality) << *(combinations.at(i)); + << (i == 0) << (type == Equality) << combinations.at(i); ++nRows_; } } @@ -1738,7 +1754,7 @@ private: QString name; Type type; int nRows_; - QList *> combinations; + QVector > > combinations; }; class IntroPage : public QWizardPage @@ -1822,9 +1838,9 @@ public: } } - void applyOperations(const QList &operations) + void applyOperations(const QVector > &operations) { - foreach (Operation * op, operations) { + foreach (const QSharedPointer &op, operations) { if (op) { op->apply(this); opsDescr += QString("(%1) ").arg(op->describe()); @@ -1844,31 +1860,29 @@ public: class CombinationsTestData { TestGroup testGroup; - QList pageOps; - QList styleOps; - QMap *> setAllOptions; + QVector > pageOps; + QVector > styleOps; + QMap > > setAllOptions; public: CombinationsTestData() { QTest::addColumn("ref"); QTest::addColumn("testEquality"); - QTest::addColumn >("operations"); - pageOps << new SetPage(0) << new SetPage(1) << new SetPage(2); - styleOps << new SetStyle(QWizard::ClassicStyle) << new SetStyle(QWizard::ModernStyle) - << new SetStyle(QWizard::MacStyle); + QTest::addColumn > >("operations"); + pageOps << SetPage::create(0) << SetPage::create(1) << SetPage::create(2); + styleOps << SetStyle::create(QWizard::ClassicStyle) << SetStyle::create(QWizard::ModernStyle) + << SetStyle::create(QWizard::MacStyle); #define SETPAGE(page) pageOps.at(page) #define SETSTYLE(style) styleOps.at(style) #define OPT(option, on) OptionInfo::instance().operation(option, on) #define CLROPT(option) OPT(option, false) #define SETOPT(option) OPT(option, true) - setAllOptions[false] = new QList; - setAllOptions[true] = new QList; foreach (QWizard::WizardOption option, OptionInfo::instance().options()) { - *setAllOptions.value(false) << CLROPT(option); - *setAllOptions.value(true) << SETOPT(option); + setAllOptions[false] << CLROPT(option); + setAllOptions[true] << SETOPT(option); } -#define CLRALLOPTS *setAllOptions.value(false) -#define SETALLOPTS *setAllOptions.value(true) +#define CLRALLOPTS setAllOptions.value(false) +#define SETALLOPTS setAllOptions.value(true) } int nRows() const { return testGroup.nRows(); } @@ -1920,7 +1934,7 @@ public: testGroup.createTestRows(); for (int i = 0; i < 2; ++i) { - QList setOptions = *setAllOptions.value(i == 1); + QVector > setOptions = setAllOptions.value(i == 1); testGroup.reset("testAll 3.1"); testGroup.add() << setOptions; @@ -1937,21 +1951,21 @@ public: testGroup.createTestRows(); } - foreach (Operation *pageOp, pageOps) { + foreach (const QSharedPointer &pageOp, pageOps) { testGroup.reset("testAll 4.1"); testGroup.add() << pageOp; testGroup.add() << pageOp << pageOp; testGroup.createTestRows(); for (int i = 0; i < 2; ++i) { - QList optionOps = *setAllOptions.value(i == 1); + QVector > optionOps = setAllOptions.value(i == 1); testGroup.reset("testAll 4.2"); testGroup.add() << optionOps << pageOp; testGroup.add() << pageOp << optionOps; testGroup.createTestRows(); foreach (QWizard::WizardOption option, OptionInfo::instance().options()) { - Operation *optionOp = OPT(option, i == 1); + QSharedPointer optionOp = OPT(option, i == 1); testGroup.reset("testAll 4.3"); testGroup.add() << optionOp << pageOp; testGroup.add() << pageOp << optionOp; @@ -1960,21 +1974,21 @@ public: } } - foreach (Operation *styleOp, styleOps) { + foreach (const QSharedPointer &styleOp, styleOps) { testGroup.reset("testAll 5.1"); testGroup.add() << styleOp; testGroup.add() << styleOp << styleOp; testGroup.createTestRows(); for (int i = 0; i < 2; ++i) { - QList optionOps = *setAllOptions.value(i == 1); + QVector > optionOps = setAllOptions.value(i == 1); testGroup.reset("testAll 5.2"); testGroup.add() << optionOps << styleOp; testGroup.add() << styleOp << optionOps; testGroup.createTestRows(); foreach (QWizard::WizardOption option, OptionInfo::instance().options()) { - Operation *optionOp = OPT(option, i == 1); + QSharedPointer optionOp = OPT(option, i == 1); testGroup.reset("testAll 5.3"); testGroup.add() << optionOp << styleOp; testGroup.add() << styleOp << optionOp; @@ -1983,8 +1997,8 @@ public: } } - foreach (Operation *pageOp, pageOps) { - foreach (Operation *styleOp, styleOps) { + foreach (const QSharedPointer &pageOp, pageOps) { + foreach (const QSharedPointer &styleOp, styleOps) { testGroup.reset("testAll 6.1"); testGroup.add() << pageOp; @@ -2002,7 +2016,7 @@ public: testGroup.createTestRows(); for (int i = 0; i < 2; ++i) { - QList optionOps = *setAllOptions.value(i == 1); + QVector > optionOps = setAllOptions.value(i == 1); testGroup.reset("testAll 6.4"); testGroup.add() << optionOps << pageOp << styleOp; testGroup.add() << pageOp << optionOps << styleOp; @@ -2013,7 +2027,7 @@ public: testGroup.createTestRows(); foreach (QWizard::WizardOption option, OptionInfo::instance().options()) { - Operation *optionOp = OPT(option, i == 1); + QSharedPointer optionOp = OPT(option, i == 1); testGroup.reset("testAll 6.5"); testGroup.add() << optionOp << pageOp << styleOp; testGroup.add() << pageOp << optionOp << styleOp; @@ -2079,7 +2093,7 @@ void tst_QWizard::combinations() QFETCH(bool, ref); QFETCH(bool, testEquality); - QFETCH(QList, operations); + QFETCH(QVector >, operations); TestWizard wizard; #if !defined(QT_NO_STYLE_WINDOWSVISTA) -- cgit v1.2.3