QCborMap: remove the optimization not to detach from non-const find()

All our tests were find() == end() or !=, which depends on the
evaluation order of the arguments to operator==(). If end() is called
first, then the detach happens before find() and all is well. But if
find() is called first, it may return end() before end() detaches.

[ChangeLog][QCborMap] Fixed a bug that could cause the iterator returned
from a failing key search with find() not to match end(). Now, every
call to find() will detach in shared QCborMaps; to avoid this, use
constFind() and constEnd().

Fixes: QTBUG-84583
Pick-to: 5.15 5.12
Change-Id: I552d244076a447ab92d7fffd161793496a8d03a8
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Thiago Macieira 2020-06-11 12:15:36 -07:00 committed by Mårten Nordheim
parent cdcb75c46b
commit 727fab7d29
2 changed files with 66 additions and 8 deletions

View File

@ -834,9 +834,8 @@ QCborValueRef QCborMap::operator[](const QCborValue &key)
*/
QCborMap::iterator QCborMap::find(qint64 key)
{
detach();
auto it = constFind(key);
if (it != constEnd())
detach();
return { d.data(), it.item.i };
}
@ -860,9 +859,8 @@ QCborMap::iterator QCborMap::find(qint64 key)
*/
QCborMap::iterator QCborMap::find(QLatin1String key)
{
detach();
auto it = constFind(key);
if (it != constEnd())
detach();
return { d.data(), it.item.i };
}
@ -886,9 +884,8 @@ QCborMap::iterator QCborMap::find(QLatin1String key)
*/
QCborMap::iterator QCborMap::find(const QString & key)
{
detach();
auto it = constFind(key);
if (it != constEnd())
detach();
return { d.data(), it.item.i };
}
@ -912,9 +909,8 @@ QCborMap::iterator QCborMap::find(const QString & key)
*/
QCborMap::iterator QCborMap::find(const QCborValue &key)
{
detach();
auto it = constFind(key);
if (it != constEnd())
detach();
return { d.data(), it.item.i };
}

View File

@ -63,6 +63,7 @@ private slots:
void arrayDefaultInitialization();
void arrayEmptyInitializerList();
void arrayEmptyDetach();
void arrayNonEmptyDetach();
void arrayInitializerList();
void arrayMutation();
void arrayMutateWithCopies();
@ -78,6 +79,7 @@ private slots:
void mapDefaultInitialization();
void mapEmptyInitializerList();
void mapEmptyDetach();
void mapNonEmptyDetach();
void mapSimpleInitializerList();
void mapMutation();
void mapMutateWithCopies();
@ -657,6 +659,66 @@ void tst_QCborValue::mapEmptyDetach()
QCOMPARE(m2, m);
}
void tst_QCborValue::arrayNonEmptyDetach()
{
QCborArray a;
a.append(1);
a.append(2);
QCOMPARE(a.first(), 1);
QCOMPARE(a.last(), 2);
QVERIFY(!a.contains(3));
QVERIFY(a.constBegin() != a.constEnd());
QVERIFY(a.begin() != a.end());
// now the same, with an active copy
{ QCborArray copy(a); QCOMPARE(a.first(), 1); }
{ QCborArray copy(a); QCOMPARE(a.last(), 2); }
{ QCborArray copy(a); QVERIFY(!a.contains(3)); }
{ QCborArray copy(a); QVERIFY(a.constBegin() != a.constEnd()); }
{ QCborArray copy(a); QVERIFY(a.begin() != a.end()); }
}
void tst_QCborValue::mapNonEmptyDetach()
{
QCborMap m;
m.insert(1, {});
m.insert(2, nullptr);
QVERIFY(!m.contains(3));
QVERIFY(m.constBegin() != m.constEnd());
QVERIFY(m.begin() != m.end());
// test all 4 overloads of find()
QVERIFY(m.constFind(3) == m.constEnd());
QVERIFY(m.constFind(QLatin1String("3")) == m.constEnd());
QVERIFY(m.constFind(QString("3")) == m.constEnd());
QVERIFY(m.constFind(QCborValue(3)) == m.constEnd());
QVERIFY(m.find(3) == m.end());
QVERIFY(m.find(QLatin1String("3")) == m.end());
QVERIFY(m.find(QString("3")) == m.end());
QVERIFY(m.find(QCborValue(3)) == m.end());
{ auto it = m.find(3); QVERIFY(it == m.end()); }
{ auto it = m.find(QLatin1String("3")); QVERIFY(it == m.end()); }
{ auto it = m.find(QString("3")); QVERIFY(it == m.end()); }
{ auto it = m.find(QCborValue(3)); QVERIFY(it == m.end()); }
// now the same, with an active copy
{ QCborMap copy(m); QVERIFY(!m.contains(3)); }
{ QCborMap copy(m); QVERIFY(m.constBegin() != m.constEnd()); }
{ QCborMap copy(m); QVERIFY(m.begin() != m.end()); }
{ QCborMap copy(m); QVERIFY(m.constFind(3) == m.constEnd()); }
{ QCborMap copy(m); QVERIFY(m.constFind(QLatin1String("3")) == m.constEnd()); }
{ QCborMap copy(m); QVERIFY(m.constFind(QString("3")) == m.constEnd()); }
{ QCborMap copy(m); QVERIFY(m.constFind(QCborValue(3)) == m.constEnd()); }
{ QCborMap copy(m); QVERIFY(m.find(3) == m.end()); }
{ QCborMap copy(m); QVERIFY(m.find(QLatin1String("3")) == m.end()); }
{ QCborMap copy(m); QVERIFY(m.find(QString("3")) == m.end()); }
{ QCborMap copy(m); QVERIFY(m.find(QCborValue(3)) == m.end()); }\
{ QCborMap copy(m); auto it = m.find(3); QVERIFY(it == m.end()); }
{ QCborMap copy(m); auto it = m.find(QLatin1String("3")); QVERIFY(it == m.end()); }
{ QCborMap copy(m); auto it = m.find(QString("3")); QVERIFY(it == m.end()); }
{ QCborMap copy(m); auto it = m.find(QCborValue(3)); QVERIFY(it == m.end()); }
}
void tst_QCborValue::arrayInitializerList()
{
QCborArray a{0, -1, false, true, nullptr, {}, 1.0};