From 2ba1d540e6175d06c21be86614704f9e56eb5d96 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Mon, 17 Aug 2020 17:31:44 +0200 Subject: QMultiMap: fix remove(Key, T) when key/value belong to the map Just like any other container, it's legitimate for the user to pass key/values belonging to the same container. Q(Multi)Map::remove(Key) are already safe (either they call erase() directly on std::(multi)map, where it does the right thing, or they skip elements while detaching). However, QMultiMap::remove(Key, T) wasn't safe in this regard (the implementation is hand rolled), so take copies before start erasing. Change-Id: I87767d608b83216a6ff264fb6c8f145fdb5934f8 Reviewed-by: Thiago Macieira --- tests/auto/corelib/tools/qmap/tst_qmap.cpp | 171 +++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) (limited to 'tests/auto/corelib/tools') diff --git a/tests/auto/corelib/tools/qmap/tst_qmap.cpp b/tests/auto/corelib/tools/qmap/tst_qmap.cpp index 4f7ac08c2d..5385471b8b 100644 --- a/tests/auto/corelib/tools/qmap/tst_qmap.cpp +++ b/tests/auto/corelib/tools/qmap/tst_qmap.cpp @@ -77,6 +77,7 @@ private slots: void testInsertWithHint(); void testInsertMultiWithHint(); void eraseValidIteratorOnSharedMap(); + void removeElementsInMap(); }; struct IdentityTracker { @@ -1649,5 +1650,175 @@ void tst_QMap::eraseValidIteratorOnSharedMap() QCOMPARE(ms3.size(), 3); } +void tst_QMap::removeElementsInMap() +{ + // A class that causes an almost certain crash if its operator< is + // called on a destroyed object + struct SharedInt { + QSharedPointer m_int; + explicit SharedInt(int i) : m_int(QSharedPointer::create(i)) {} + bool operator<(const SharedInt &other) const { return *m_int < *other.m_int; } + }; + + { + QMap map { + { SharedInt(1), 1 }, + { SharedInt(2), 2 }, + { SharedInt(3), 3 }, + { SharedInt(4), 4 }, + { SharedInt(5), 5 }, + }; + QCOMPARE(map.size(), 5); + + map.remove(SharedInt(1)); + QCOMPARE(map.size(), 4); + + map.remove(SharedInt(-1)); + QCOMPARE(map.size(), 4); + + QMap map2 = map; + QCOMPARE(map.size(), 4); + QCOMPARE(map2.size(), 4); + + map.remove(SharedInt(3)); + QCOMPARE(map.size(), 3); + QCOMPARE(map2.size(), 4); + + map.remove(SharedInt(-1)); + QCOMPARE(map.size(), 3); + QCOMPARE(map2.size(), 4); + + map = map2; + QCOMPARE(map.size(), 4); + QCOMPARE(map2.size(), 4); + + map.remove(SharedInt(-1)); + QCOMPARE(map.size(), 4); + QCOMPARE(map2.size(), 4); + + map.remove(map.firstKey()); + QCOMPARE(map.size(), 3); + QCOMPARE(map2.size(), 4); + + map.remove(map.lastKey()); + QCOMPARE(map.size(), 2); + QCOMPARE(map2.size(), 4); + + map = map2; + QCOMPARE(map.size(), 4); + QCOMPARE(map2.size(), 4); + + auto size = map.size(); + for (auto it = map.begin(); it != map.end(); ) { + const auto oldIt = it++; + size -= map.remove(oldIt.key()); + QCOMPARE(map.size(), size); + QCOMPARE(map2.size(), 4); + } + + QCOMPARE(map.size(), 0); + QCOMPARE(map2.size(), 4); + } + + { + QMultiMap multimap { + { SharedInt(1), 10 }, + { SharedInt(1), 11 }, + { SharedInt(2), 2 }, + { SharedInt(3), 30 }, + { SharedInt(3), 31 }, + { SharedInt(3), 32 }, + { SharedInt(4), 4 }, + { SharedInt(5), 5 }, + { SharedInt(6), 60 }, + { SharedInt(6), 61 }, + { SharedInt(6), 60 }, + { SharedInt(6), 62 }, + { SharedInt(6), 60 }, + { SharedInt(7), 7 }, + }; + + QCOMPARE(multimap.size(), 14); + + multimap.remove(SharedInt(1)); + QCOMPARE(multimap.size(), 12); + + multimap.remove(SharedInt(-1)); + QCOMPARE(multimap.size(), 12); + + QMultiMap multimap2 = multimap; + QCOMPARE(multimap.size(), 12); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(SharedInt(3)); + QCOMPARE(multimap.size(), 9); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(SharedInt(4)); + QCOMPARE(multimap.size(), 8); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(SharedInt(-1)); + QCOMPARE(multimap.size(), 8); + QCOMPARE(multimap2.size(), 12); + + multimap = multimap2; + QCOMPARE(multimap.size(), 12); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(SharedInt(-1)); + QCOMPARE(multimap.size(), 12); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(SharedInt(6), 60); + QCOMPARE(multimap.size(), 9); + QCOMPARE(multimap2.size(), 12); + + multimap = multimap2; + QCOMPARE(multimap.size(), 12); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(SharedInt(6), 62); + QCOMPARE(multimap.size(), 11); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(multimap.firstKey()); + QCOMPARE(multimap.size(), 10); + QCOMPARE(multimap2.size(), 12); + + multimap.remove(multimap.lastKey()); + QCOMPARE(multimap.size(), 9); + QCOMPARE(multimap2.size(), 12); + + multimap = multimap2; + QCOMPARE(multimap.size(), 12); + QCOMPARE(multimap2.size(), 12); + + auto itFor6 = multimap.find(SharedInt(6)); + QVERIFY(itFor6 != multimap.end()); + QCOMPARE(itFor6.value(), 60); + multimap.remove(itFor6.key(), itFor6.value()); + QCOMPARE(multimap.size(), 9); + QCOMPARE(multimap2.size(), 12); + + multimap = multimap2; + QCOMPARE(multimap.size(), 12); + QCOMPARE(multimap2.size(), 12); + + auto size = multimap.size(); + for (auto it = multimap.begin(); it != multimap.end();) { + const auto range = multimap.equal_range(it.key()); + const auto oldIt = it; + it = range.second; + size -= multimap.remove(oldIt.key()); + QCOMPARE(multimap.size(), size); + QCOMPARE(multimap2.size(), 12); + } + + QCOMPARE(multimap.size(), 0); + QCOMPARE(multimap2.size(), 12); + } +} + QTEST_APPLESS_MAIN(tst_QMap) #include "tst_qmap.moc" -- cgit v1.2.3