From 0ff75e7a882523d49c5bf74a32fcb68d0d7b1afc Mon Sep 17 00:00:00 2001 From: Michal Kaptur Date: Mon, 27 Nov 2017 21:41:58 +0100 Subject: [PATCH] Fixed memory leaks in non-gui tests Fixed 2 memory leaks in production code and a few in testcases. As a result leak_check_at_exit ASAN option does not need to turned off for non-gui tests. Smart pointers should be used elsewhere for consistency, but the sooner this fixes are delivered, the lesser memory leaks are introduced. --- .travis.yml | 6 +- src/crypto/SymmetricCipherGcrypt.cpp | 2 + src/format/KeePass2Repair.cpp | 37 +++----- src/format/KeePass2Repair.h | 8 +- src/gui/DatabaseRepairWidget.cpp | 6 +- tests/TestCsvParser.cpp | 87 +++++++++--------- tests/TestCsvParser.h | 6 +- tests/TestGroup.cpp | 132 +++++++++++---------------- tests/TestKeePass2Writer.cpp | 6 +- tests/TestSymmetricCipher.cpp | 2 +- 10 files changed, 125 insertions(+), 167 deletions(-) diff --git a/.travis.yml b/.travis.yml index df7f8ed58..942bb426d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,8 +15,8 @@ compiler: - gcc env: - - CONFIG=Release ASAN_OPTIONS=detect_odr_violation=1:leak_check_at_exit=0 - - CONFIG=Debug ASAN_OPTIONS=detect_odr_violation=1:leak_check_at_exit=0 + - CONFIG=Release ASAN_OPTIONS=detect_odr_violation=1 + - CONFIG=Debug ASAN_OPTIONS=detect_odr_violation=1 git: depth: 3 @@ -37,7 +37,7 @@ script: - cmake -DCMAKE_BUILD_TYPE=${CONFIG} -DWITH_GUI_TESTS=ON -DWITH_ASAN=ON -DWITH_XC_HTTP=ON -DWITH_XC_AUTOTYPE=ON -DWITH_XC_YUBIKEY=ON $CMAKE_ARGS .. - make -j2 - if [ "$TRAVIS_OS_NAME" = "linux" ]; then make test ARGS+="-E testgui --output-on-failure"; fi - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then xvfb-run -a --server-args="-screen 0 800x600x24" make test ARGS+="-R testgui --output-on-failure"; fi + - if [ "$TRAVIS_OS_NAME" = "linux" ]; then ASAN_OPTIONS=${ASAN_OPTIONS}:leak_check_at_exit=0 xvfb-run -a --server-args="-screen 0 800x600x24" make test ARGS+="-R testgui --output-on-failure"; fi - if [ "$TRAVIS_OS_NAME" = "osx" ]; then make test ARGS+="--output-on-failure"; fi # Generate snapcraft build when merging into master/develop branches diff --git a/src/crypto/SymmetricCipherGcrypt.cpp b/src/crypto/SymmetricCipherGcrypt.cpp index e600a7edb..1805afb0b 100644 --- a/src/crypto/SymmetricCipherGcrypt.cpp +++ b/src/crypto/SymmetricCipherGcrypt.cpp @@ -86,6 +86,8 @@ bool SymmetricCipherGcrypt::init() gcry_error_t error; + if(m_ctx != nullptr) + gcry_cipher_close(m_ctx); error = gcry_cipher_open(&m_ctx, m_algo, m_mode, 0); if (error != 0) { setErrorString(error); diff --git a/src/format/KeePass2Repair.cpp b/src/format/KeePass2Repair.cpp index 81ada2fda..8d18457d4 100644 --- a/src/format/KeePass2Repair.cpp +++ b/src/format/KeePass2Repair.cpp @@ -1,5 +1,6 @@ /* * Copyright (C) 2016 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -18,36 +19,29 @@ #include "KeePass2Repair.h" #include +#include #include #include "format/KeePass2RandomStream.h" #include "format/KeePass2Reader.h" #include "format/KeePass2XmlReader.h" -KeePass2Repair::KeePass2Repair() - : m_db(nullptr) +KeePass2Repair::RepairOutcome KeePass2Repair::repairDatabase(QIODevice* device, const CompositeKey& key) { -} - -KeePass2Repair::RepairResult KeePass2Repair::repairDatabase(QIODevice* device, const CompositeKey& key) -{ - m_db = nullptr; m_errorStr.clear(); KeePass2Reader reader; reader.setSaveXml(true); - Database* db = reader.readDatabase(device, key, true); + QScopedPointer db(reader.readDatabase(device, key, true)); if (!reader.hasError()) { - delete db; - return NothingTodo; + return qMakePair(NothingTodo, nullptr); } QByteArray xmlData = reader.xmlData(); if (!db || xmlData.isEmpty()) { - delete db; m_errorStr = reader.errorString(); - return UnableToOpen; + return qMakePair(UnableToOpen, nullptr); } bool repairAction = false; @@ -59,8 +53,7 @@ KeePass2Repair::RepairResult KeePass2Repair::repairDatabase(QIODevice* device, c && encodingRegExp.cap(1).compare("utf8", Qt::CaseInsensitive) != 0) { // database is not utf-8 encoded, we don't support repairing that - delete db; - return RepairFailed; + return qMakePair(RepairFailed, nullptr); } } @@ -75,8 +68,7 @@ KeePass2Repair::RepairResult KeePass2Repair::repairDatabase(QIODevice* device, c if (!repairAction) { // we were unable to find the problem - delete db; - return RepairFailed; + return qMakePair(RepairFailed, nullptr); } KeePass2RandomStream randomStream; @@ -84,23 +76,16 @@ KeePass2Repair::RepairResult KeePass2Repair::repairDatabase(QIODevice* device, c KeePass2XmlReader xmlReader; QBuffer buffer(&xmlData); buffer.open(QIODevice::ReadOnly); - xmlReader.readDatabase(&buffer, db, &randomStream); + xmlReader.readDatabase(&buffer, db.data(), &randomStream); if (xmlReader.hasError()) { - delete db; - return RepairFailed; + return qMakePair(RepairFailed, nullptr); } else { - m_db = db; - return RepairSuccess; + return qMakePair(RepairSuccess, db.take()); } } -Database* KeePass2Repair::database() const -{ - return m_db; -} - QString KeePass2Repair::errorString() const { return m_errorStr; diff --git a/src/format/KeePass2Repair.h b/src/format/KeePass2Repair.h index fe2f9dbfe..e7f2c8435 100644 --- a/src/format/KeePass2Repair.h +++ b/src/format/KeePass2Repair.h @@ -1,5 +1,6 @@ /* * Copyright (C) 2016 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -20,6 +21,7 @@ #include #include +#include #include "core/Database.h" #include "keys/CompositeKey.h" @@ -36,14 +38,12 @@ public: RepairSuccess, RepairFailed }; + using RepairOutcome = QPair; - KeePass2Repair(); - RepairResult repairDatabase(QIODevice* device, const CompositeKey& key); - Database* database() const; + RepairOutcome repairDatabase(QIODevice* device, const CompositeKey& key); QString errorString() const; private: - Database* m_db; QString m_errorStr; }; diff --git a/src/gui/DatabaseRepairWidget.cpp b/src/gui/DatabaseRepairWidget.cpp index 2b0039408..d3dddf14f 100644 --- a/src/gui/DatabaseRepairWidget.cpp +++ b/src/gui/DatabaseRepairWidget.cpp @@ -1,5 +1,6 @@ /* * Copyright (C) 2016 Felix Geyer + * Copyright (C) 2017 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -69,7 +70,8 @@ void DatabaseRepairWidget::openDatabase() delete m_db; } QApplication::setOverrideCursor(QCursor(Qt::WaitCursor)); - KeePass2Repair::RepairResult repairResult = repair.repairDatabase(&file, masterKey); + auto repairOutcome = repair.repairDatabase(&file, masterKey); + KeePass2Repair::RepairResult repairResult = repairOutcome.first; QApplication::restoreOverrideCursor(); switch (repairResult) { @@ -83,7 +85,7 @@ void DatabaseRepairWidget::openDatabase() emit editFinished(false); return; case KeePass2Repair::RepairSuccess: - m_db = repair.database(); + m_db = repairOutcome.second; MessageBox::warning(this, tr("Success"), tr("The database has been successfully repaired\nYou can now save it.")); emit editFinished(true); return; diff --git a/tests/TestCsvParser.cpp b/tests/TestCsvParser.cpp index 57bc683a2..a292b56bb 100644 --- a/tests/TestCsvParser.cpp +++ b/tests/TestCsvParser.cpp @@ -24,17 +24,12 @@ QTEST_GUILESS_MAIN(TestCsvParser) void TestCsvParser::initTestCase() { - parser = new CsvParser(); -} - -void TestCsvParser::cleanupTestCase() -{ - delete parser; + parser.reset(new CsvParser()); } void TestCsvParser::init() { - file = new QTemporaryFile(); + file.reset(new QTemporaryFile()); if (not file->open()) QFAIL("Cannot open file!"); parser->setBackslashSyntax(false); @@ -51,20 +46,20 @@ void TestCsvParser::cleanup() /****************** TEST CASES ******************/ void TestCsvParser::testMissingQuote() { parser->setTextQualifier(':'); - QTextStream out(file); + QTextStream out(file.data()); out << "A,B\n:BM,1"; QEXPECT_FAIL("", "Bad format", Continue); - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QWARN(parser->getStatus().toLatin1()); } void TestCsvParser::testMalformed() { parser->setTextQualifier(':'); - QTextStream out(file); + QTextStream out(file.data()); out << "A,B,C\n:BM::,1,:2:"; QEXPECT_FAIL("", "Bad format", Continue); - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QWARN(parser->getStatus().toLatin1()); } @@ -72,14 +67,14 @@ void TestCsvParser::testMalformed() { void TestCsvParser::testBackslashSyntax() { parser->setBackslashSyntax(true); parser->setTextQualifier(QChar('X')); - QTextStream out(file); + QTextStream out(file.data()); //attended result: one"\t\"wo out << "Xone\\\"\\\\t\\\\\\\"w\noX\n" << "X13X,X2\\X,X,\"\"3\"X\r" << "3,X\"4\"X,,\n" << "XX\n" << "\\"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.at(0).at(0) == "one\"\\t\\\"w\no"); QVERIFY(t.at(1).at(0) == "13"); @@ -94,10 +89,10 @@ void TestCsvParser::testBackslashSyntax() { } void TestCsvParser::testQuoted() { - QTextStream out(file); + QTextStream out(file.data()); out << "ro,w,\"end, of \"\"\"\"\"\"row\"\"\"\"\"\n" << "2\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.at(0).at(0) == "ro"); QVERIFY(t.at(0).at(1) == "w"); @@ -107,41 +102,41 @@ void TestCsvParser::testQuoted() { } void TestCsvParser::testEmptySimple() { - QTextStream out(file); + QTextStream out(file.data()); out <<""; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 0); } void TestCsvParser::testEmptyQuoted() { - QTextStream out(file); + QTextStream out(file.data()); out <<"\"\""; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 0); } void TestCsvParser::testEmptyNewline() { - QTextStream out(file); + QTextStream out(file.data()); out <<"\"\n\""; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 0); } void TestCsvParser::testEmptyFile() { - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 0); } void TestCsvParser::testNewline() { - QTextStream out(file); + QTextStream out(file.data()); out << "1,2\n\n\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 1); QVERIFY(t.at(0).at(0) == "1"); @@ -150,9 +145,9 @@ void TestCsvParser::testNewline() void TestCsvParser::testCR() { - QTextStream out(file); + QTextStream out(file.data()); out << "1,2\r3,4"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 2); QVERIFY(t.at(0).at(0) == "1"); @@ -163,9 +158,9 @@ void TestCsvParser::testCR() void TestCsvParser::testLF() { - QTextStream out(file); + QTextStream out(file.data()); out << "1,2\n3,4"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 2); QVERIFY(t.at(0).at(0) == "1"); @@ -176,9 +171,9 @@ void TestCsvParser::testLF() void TestCsvParser::testCRLF() { - QTextStream out(file); + QTextStream out(file.data()); out << "1,2\r\n3,4"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 2); QVERIFY(t.at(0).at(0) == "1"); @@ -189,13 +184,13 @@ void TestCsvParser::testCRLF() void TestCsvParser::testComments() { - QTextStream out(file); + QTextStream out(file.data()); out << " #one\n" << " \t # two, three \r\n" << " #, sing\t with\r" << " #\t me!\n" << "useful,text #1!"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 1); QVERIFY(t.at(0).at(0) == "useful"); @@ -203,21 +198,21 @@ void TestCsvParser::testComments() } void TestCsvParser::testColumns() { - QTextStream out(file); + QTextStream out(file.data()); out << "1,2\n" << ",,,,,,,,,a\n" << "a,b,c,d\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(parser->getCsvCols() == 10); } void TestCsvParser::testSimple() { - QTextStream out(file); + QTextStream out(file.data()); out << ",,2\r,2,3\n" << "A,,B\"\n" << " ,,\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 4); QVERIFY(t.at(0).at(0) == ""); @@ -236,11 +231,11 @@ void TestCsvParser::testSimple() { void TestCsvParser::testSeparator() { parser->setFieldSeparator('\t'); - QTextStream out(file); + QTextStream out(file.data()); out << "\t\t2\r\t2\t3\n" << "A\t\tB\"\n" << " \t\t\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 4); QVERIFY(t.at(0).at(0) == ""); @@ -260,10 +255,10 @@ void TestCsvParser::testSeparator() { void TestCsvParser::testMultiline() { parser->setTextQualifier(QChar(':')); - QTextStream out(file); + QTextStream out(file.data()); out << ":1\r\n2a::b:,:3\r4:\n" << "2\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.at(0).at(0) == "1\n2a:b"); QVERIFY(t.at(0).at(1) == "3\n4"); @@ -281,10 +276,10 @@ void TestCsvParser::testEmptyReparsing() void TestCsvParser::testReparsing() { - QTextStream out(file); + QTextStream out(file.data()); out << ":te\r\nxt1:,:te\rxt2:,:end of \"this\n string\":\n" << "2\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QEXPECT_FAIL("", "Wrong qualifier", Continue); @@ -303,10 +298,10 @@ void TestCsvParser::testReparsing() void TestCsvParser::testQualifier() { parser->setTextQualifier(QChar('X')); - QTextStream out(file); + QTextStream out(file.data()); out << "X1X,X2XX,X,\"\"3\"\"\"X\r" << "3,X\"4\"X,,\n"; - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 2); QVERIFY(t.at(0).at(0) == "1"); @@ -324,10 +319,10 @@ void TestCsvParser::testUnicode() { //CORRECT QChar g(0x20AC); //ERROR QChar g("\u20AC"); parser->setFieldSeparator(QChar('A')); - QTextStream out(file); + QTextStream out(file.data()); out << QString("€1A2śA\"3śAż\"Ażac"); - QVERIFY(parser->parse(file)); + QVERIFY(parser->parse(file.data())); t = parser->getCsvTable(); QVERIFY(t.size() == 1); QVERIFY(t.at(0).at(0) == "€1"); diff --git a/tests/TestCsvParser.h b/tests/TestCsvParser.h index 0cf8b94d3..f8c327d63 100644 --- a/tests/TestCsvParser.h +++ b/tests/TestCsvParser.h @@ -22,6 +22,7 @@ #include #include #include +#include #include "core/CsvParser.h" @@ -37,7 +38,6 @@ private slots: void init(); void cleanup(); void initTestCase(); - void cleanupTestCase(); void testUnicode(); void testLF(); @@ -62,8 +62,8 @@ private slots: void testColumns(); private: - QTemporaryFile* file; - CsvParser* parser; + QScopedPointer file; + QScopedPointer parser; CsvTable t; void dumpRow(CsvTable table, int row); }; diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index 50997dcca..dfccf5d0b 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -18,9 +18,10 @@ #include "TestGroup.h" -#include -#include #include +#include +#include +#include #include #include "core/Database.h" @@ -75,6 +76,7 @@ void TestGroup::testParenting() QCOMPARE(g3->children().size(), 1); QCOMPARE(g4->children().size(), 0); + QVERIFY(rootGroup->children().at(0) == g1); QVERIFY(rootGroup->children().at(0) == g1); QVERIFY(g1->children().at(0) == g2); QVERIFY(g1->children().at(1) == g3); @@ -99,7 +101,6 @@ void TestGroup::testParenting() g3->setIcon(Uuid::random()); g1->setIcon(2); QCOMPARE(spy.count(), 6); - delete db; QVERIFY(rootGroup.isNull()); @@ -107,7 +108,6 @@ void TestGroup::testParenting() QVERIFY(g2.isNull()); QVERIFY(g3.isNull()); QVERIFY(g4.isNull()); - delete tmpRoot; } @@ -117,18 +117,18 @@ void TestGroup::testSignals() Database* db2 = new Database(); QPointer root = db->rootGroup(); - QSignalSpy spyAboutToAdd(db, SIGNAL(groupAboutToAdd(Group*,int))); + QSignalSpy spyAboutToAdd(db, SIGNAL(groupAboutToAdd(Group*, int))); QSignalSpy spyAdded(db, SIGNAL(groupAdded())); QSignalSpy spyAboutToRemove(db, SIGNAL(groupAboutToRemove(Group*))); QSignalSpy spyRemoved(db, SIGNAL(groupRemoved())); - QSignalSpy spyAboutToMove(db, SIGNAL(groupAboutToMove(Group*,Group*,int))); + QSignalSpy spyAboutToMove(db, SIGNAL(groupAboutToMove(Group*, Group*, int))); QSignalSpy spyMoved(db, SIGNAL(groupMoved())); - QSignalSpy spyAboutToAdd2(db2, SIGNAL(groupAboutToAdd(Group*,int))); + QSignalSpy spyAboutToAdd2(db2, SIGNAL(groupAboutToAdd(Group*, int))); QSignalSpy spyAdded2(db2, SIGNAL(groupAdded())); QSignalSpy spyAboutToRemove2(db2, SIGNAL(groupAboutToRemove(Group*))); QSignalSpy spyRemoved2(db2, SIGNAL(groupRemoved())); - QSignalSpy spyAboutToMove2(db2, SIGNAL(groupAboutToMove(Group*,Group*,int))); + QSignalSpy spyAboutToMove2(db2, SIGNAL(groupAboutToMove(Group*, Group*, int))); QSignalSpy spyMoved2(db2, SIGNAL(groupMoved())); Group* g1 = new Group(); @@ -251,7 +251,7 @@ void TestGroup::testEntries() void TestGroup::testDeleteSignals() { - Database* db = new Database(); + QScopedPointer db(new Database()); Group* groupRoot = db->rootGroup(); Group* groupChild = new Group(); Group* groupChildChild = new Group(); @@ -260,15 +260,13 @@ void TestGroup::testDeleteSignals() groupChildChild->setObjectName("groupChildChild"); groupChild->setParent(groupRoot); groupChildChild->setParent(groupChild); - QSignalSpy spyAboutToRemove(db, SIGNAL(groupAboutToRemove(Group*))); - QSignalSpy spyRemoved(db, SIGNAL(groupRemoved())); + QSignalSpy spyAboutToRemove(db.data(), SIGNAL(groupAboutToRemove(Group*))); + QSignalSpy spyRemoved(db.data(), SIGNAL(groupRemoved())); delete groupChild; QVERIFY(groupRoot->children().isEmpty()); QCOMPARE(spyAboutToRemove.count(), 2); QCOMPARE(spyRemoved.count(), 2); - delete db; - Group* group = new Group(); Entry* entry = new Entry(); @@ -282,7 +280,7 @@ void TestGroup::testDeleteSignals() QCOMPARE(spyEntryRemoved.count(), 1); delete group; - Database* db2 = new Database(); + QScopedPointer db2(new Database()); Group* groupRoot2 = db2->rootGroup(); Group* group2 = new Group(); group2->setParent(groupRoot2); @@ -294,12 +292,11 @@ void TestGroup::testDeleteSignals() delete group2; QCOMPARE(spyEntryAboutToRemove2.count(), 1); QCOMPARE(spyEntryRemoved2.count(), 1); - delete db2; } void TestGroup::testCopyCustomIcon() { - Database* dbSource = new Database(); + QScopedPointer dbSource(new Database()); Uuid groupIconUuid = Uuid::random(); QImage groupIcon(16, 16, QImage::Format_RGB32); @@ -321,7 +318,7 @@ void TestGroup::testCopyCustomIcon() entry->setIcon(entryIconUuid); QCOMPARE(entry->icon(), entryIcon); - Database* dbTarget = new Database(); + QScopedPointer dbTarget(new Database()); group->setParent(dbTarget->rootGroup()); QVERIFY(dbTarget->metadata()->containsCustomIcon(groupIconUuid)); @@ -332,37 +329,34 @@ void TestGroup::testCopyCustomIcon() QVERIFY(dbTarget->metadata()->containsCustomIcon(entryIconUuid)); QCOMPARE(dbTarget->metadata()->customIcon(entryIconUuid), entryIcon); QCOMPARE(entry->icon(), entryIcon); - - delete dbSource; - delete dbTarget; } void TestGroup::testClone() { - Database* db = new Database(); + QScopedPointer db(new Database()); - Group* originalGroup = new Group(); + QScopedPointer originalGroup(new Group()); originalGroup->setParent(db->rootGroup()); originalGroup->setName("Group"); originalGroup->setIcon(42); - Entry* originalGroupEntry = new Entry(); - originalGroupEntry->setGroup(originalGroup); + QScopedPointer originalGroupEntry(new Entry()); + originalGroupEntry->setGroup(originalGroup.data()); originalGroupEntry->setTitle("GroupEntryOld"); originalGroupEntry->setIcon(43); originalGroupEntry->beginUpdate(); originalGroupEntry->setTitle("GroupEntry"); originalGroupEntry->endUpdate(); - Group* subGroup = new Group(); - subGroup->setParent(originalGroup); + QScopedPointer subGroup(new Group()); + subGroup->setParent(originalGroup.data()); subGroup->setName("SubGroup"); - Entry* subGroupEntry = new Entry(); - subGroupEntry->setGroup(subGroup); + QScopedPointer subGroupEntry(new Entry()); + subGroupEntry->setGroup(subGroup.data()); subGroupEntry->setTitle("SubGroupEntry"); - Group* clonedGroup = originalGroup->clone(); + QScopedPointer clonedGroup(originalGroup->clone()); QVERIFY(!clonedGroup->parentGroup()); QVERIFY(!clonedGroup->database()); QVERIFY(clonedGroup->uuid() != originalGroup->uuid()); @@ -387,19 +381,15 @@ void TestGroup::testClone() QVERIFY(clonedSubGroupEntry->uuid() != subGroupEntry->uuid()); QCOMPARE(clonedSubGroupEntry->title(), QString("SubGroupEntry")); - Group* clonedGroupKeepUuid = originalGroup->clone(Entry::CloneNoFlags); + QScopedPointer clonedGroupKeepUuid(originalGroup->clone(Entry::CloneNoFlags)); QCOMPARE(clonedGroupKeepUuid->entries().at(0)->uuid(), originalGroupEntry->uuid()); QCOMPARE(clonedGroupKeepUuid->children().at(0)->entries().at(0)->uuid(), subGroupEntry->uuid()); - - delete clonedGroup; - delete clonedGroupKeepUuid; - delete db; } void TestGroup::testCopyCustomIcons() { - Database* dbSource = new Database(); - Database* dbTarget = new Database(); + QScopedPointer dbSource(new Database()); + QScopedPointer dbTarget(new Database()); QImage iconImage1(1, 1, QImage::Format_RGB32); iconImage1.setPixel(0, 0, qRgb(1, 2, 3)); @@ -407,20 +397,20 @@ void TestGroup::testCopyCustomIcons() QImage iconImage2(1, 1, QImage::Format_RGB32); iconImage2.setPixel(0, 0, qRgb(4, 5, 6)); - Group* group1 = new Group(); + QScopedPointer group1(new Group()); group1->setParent(dbSource->rootGroup()); Uuid group1Icon = Uuid::random(); dbSource->metadata()->addCustomIcon(group1Icon, iconImage1); group1->setIcon(group1Icon); - Group* group2 = new Group(); - group2->setParent(group1); + QScopedPointer group2(new Group()); + group2->setParent(group1.data()); Uuid group2Icon = Uuid::random(); dbSource->metadata()->addCustomIcon(group2Icon, iconImage1); group2->setIcon(group2Icon); - Entry* entry1 = new Entry(); - entry1->setGroup(group2); + QScopedPointer entry1(new Entry()); + entry1->setGroup(group2.data()); Uuid entry1IconOld = Uuid::random(); dbSource->metadata()->addCustomIcon(entry1IconOld, iconImage1); entry1->setIcon(entry1IconOld); @@ -447,27 +437,24 @@ void TestGroup::testCopyCustomIcons() QCOMPARE(metaTarget->customIcon(group1Icon).pixel(0, 0), qRgb(1, 2, 3)); QCOMPARE(metaTarget->customIcon(group2Icon).pixel(0, 0), qRgb(4, 5, 6)); - - delete dbTarget; - delete dbSource; } void TestGroup::testMerge() { - Group* group1 = new Group(); + QScopedPointer group1(new Group()); group1->setName("group 1"); - Group* group2 = new Group(); + QScopedPointer group2(new Group()); group2->setName("group 2"); - Entry* entry1 = new Entry(); - Entry* entry2 = new Entry(); + QScopedPointer entry1(new Entry()); + QScopedPointer entry2(new Entry()); - entry1->setGroup(group1); + entry1->setGroup(group1.data()); entry1->setUuid(Uuid::random()); - entry2->setGroup(group1); + entry2->setGroup(group1.data()); entry2->setUuid(Uuid::random()); - group2->merge(group1); + group2->merge(group1.data()); QCOMPARE(group1->entries().size(), 2); QCOMPARE(group2->entries().size(), 2); @@ -475,25 +462,22 @@ void TestGroup::testMerge() void TestGroup::testMergeDatabase() { - Database* dbSource = createMergeTestDatabase(); - Database* dbDest = new Database(); + QScopedPointer dbSource(createMergeTestDatabase()); + QScopedPointer dbDest(new Database()); - dbDest->merge(dbSource); + dbDest->merge(dbSource.data()); QCOMPARE(dbDest->rootGroup()->children().size(), 2); QCOMPARE(dbDest->rootGroup()->children().at(0)->entries().size(), 2); - - delete dbDest; - delete dbSource; } void TestGroup::testMergeConflict() { - Database* dbSource = createMergeTestDatabase(); + QScopedPointer dbSource(createMergeTestDatabase()); // test merging updated entries // falls back to KeepBoth mode - Database* dbCopy = new Database(); + QScopedPointer dbCopy(new Database()); dbCopy->setRootGroup(dbSource->rootGroup()->clone(Entry::CloneNoFlags)); // sanity check @@ -505,22 +489,19 @@ void TestGroup::testMergeConflict() updatedTimeInfo.setLastModificationTime(updatedTimeInfo.lastModificationTime().addYears(1)); updatedEntry->setTimeInfo(updatedTimeInfo); - dbCopy->merge(dbSource); + dbCopy->merge(dbSource.data()); // one entry is duplicated because of mode QCOMPARE(dbCopy->rootGroup()->children().at(0)->entries().size(), 2); - - delete dbSource; - delete dbCopy; } void TestGroup::testMergeConflictKeepBoth() { - Database* dbSource = createMergeTestDatabase(); + QScopedPointer dbSource(createMergeTestDatabase()); // test merging updated entries // falls back to KeepBoth mode - Database* dbCopy = new Database(); + QScopedPointer dbCopy(new Database()); dbCopy->setRootGroup(dbSource->rootGroup()->clone(Entry::CloneNoFlags)); // sanity check @@ -534,21 +515,18 @@ void TestGroup::testMergeConflictKeepBoth() dbCopy->rootGroup()->setMergeMode(Group::MergeMode::KeepBoth); - dbCopy->merge(dbSource); + dbCopy->merge(dbSource.data()); // one entry is duplicated because of mode QCOMPARE(dbCopy->rootGroup()->children().at(0)->entries().size(), 3); // the older entry was merged from the other db as last in the group Entry* olderEntry = dbCopy->rootGroup()->children().at(0)->entries().at(2); QVERIFY2(olderEntry->attributes()->hasKey("merged"), "older entry is marked with an attribute \"merged\""); - - delete dbSource; - delete dbCopy; } Database* TestGroup::createMergeTestDatabase() { - Database* db = new Database(); + QScopedPointer db(new Database()); Group* group1 = new Group(); group1->setName("group 1"); @@ -566,12 +544,12 @@ Database* TestGroup::createMergeTestDatabase() group1->setParent(db->rootGroup()); group2->setParent(db->rootGroup()); - return db; + return db.take(); } void TestGroup::testFindEntry() { - Database* db = new Database(); + QScopedPointer db(new Database()); Entry* entry1 = new Entry(); entry1->setTitle(QString("entry1")); @@ -642,13 +620,11 @@ void TestGroup::testFindEntry() // An invalid UUID. entry = db->rootGroup()->findEntry(QString("febfb01ebcdf9dbd90a3f1579dc")); QVERIFY(entry == nullptr); - - delete db; } void TestGroup::testFindGroupByPath() { - Database* db = new Database(); + QScopedPointer db(new Database()); Group* group1 = new Group(); group1->setName("group1"); @@ -706,13 +682,11 @@ void TestGroup::testFindGroupByPath() group = db->rootGroup()->findGroupByPath("invalid"); QVERIFY(group == nullptr); - - delete db; } void TestGroup::testPrint() { - Database* db = new Database(); + QScopedPointer db(new Database()); QString output = db->rootGroup()->print(); QCOMPARE(output, QString("[empty]\n")); @@ -731,7 +705,6 @@ void TestGroup::testPrint() output = db->rootGroup()->print(true); QCOMPARE(output, QString("entry1 " + entry1->uuid().toHex() + "\n")); - Group* group1 = new Group(); group1->setName("group1"); @@ -752,5 +725,4 @@ void TestGroup::testPrint() QVERIFY(output.contains(QString("entry1 " + entry1->uuid().toHex() + "\n"))); QVERIFY(output.contains(QString("group1/ " + group1->uuid().toHex() + "\n"))); QVERIFY(output.contains(QString(" entry2 " + entry2->uuid().toHex() + "\n"))); - delete db; } diff --git a/tests/TestKeePass2Writer.cpp b/tests/TestKeePass2Writer.cpp index 9f0c87be7..f6d3f58ad 100644 --- a/tests/TestKeePass2Writer.cpp +++ b/tests/TestKeePass2Writer.cpp @@ -148,13 +148,15 @@ void TestKeePass2Writer::testRepair() KeePass2Repair repair; QFile file(brokenDbFilename); file.open(QIODevice::ReadOnly); - QCOMPARE(repair.repairDatabase(&file, key), KeePass2Repair::RepairSuccess); - Database* dbRepaired = repair.database(); + auto result = repair.repairDatabase(&file, key); + QCOMPARE(result.first, KeePass2Repair::RepairSuccess); + Database* dbRepaired = result.second; QVERIFY(dbRepaired); QCOMPARE(dbRepaired->rootGroup()->entries().size(), 1); QCOMPARE(dbRepaired->rootGroup()->entries().at(0)->username(), QString("testuser").append(QChar(0x20AC))); QCOMPARE(dbRepaired->rootGroup()->entries().at(0)->password(), QString("testpw")); + delete dbRepaired; } void TestKeePass2Writer::cleanupTestCase() diff --git a/tests/TestSymmetricCipher.cpp b/tests/TestSymmetricCipher.cpp index 4f78693d6..5242c3888 100644 --- a/tests/TestSymmetricCipher.cpp +++ b/tests/TestSymmetricCipher.cpp @@ -162,7 +162,7 @@ void TestSymmetricCipher::testTwofish256CbcEncryption() bool ok; for (int i = 0; i < keys.size(); ++i) { - cipher.init(keys[i], ivs[i]); + QVERIFY(cipher.init(keys[i], ivs[i])); QByteArray ptNext = plainTexts[i]; QByteArray ctPrev = ivs[i]; QByteArray ctCur;