whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QJsonObject::erase: erase unnecessary and wrong code 

Commit 35adb74ddd ("Reimplement JSON
support on top of Cbor") accidentally forgot to multiply by 2 the index
stored in the QJsonObject::iterator. The same mistake was propagated
when QJsonObject::iterator was converted to QJsonValueRef. This had no
ill effects because the o->elements container would always contain more
elements, but it meant the check was ineffective and meant nothing.

So instead of doing nothing when the iterator does not point to this
container, simply assume it does. Bad things will happen if you try to
erase an iterator that points to another container, but that's true for
almost all container/iterator mechanisms.

Drive-by modernization of some of the surrounding lines.

Change-Id: I89446ea06b5742efb194fffd16bb7c322c2fc4f2
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
bb10
Thiago Macieira 2021-11-27 10:57:24 -08:00
parent f276f9ec6b
commit e99417106f
2 changed files with 66 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/corelib/serialization/qjsonobject.cpp
View File

@ -707,13 +707,12 @@ bool QJsonObject::operator!=(const QJsonObject &other) const
*/
QJsonObject::iterator QJsonObject::erase(QJsonObject::iterator it)
{
if (it.item.o != this || qsizetype(it.item.index) >= o->elements.length())
return {this, o->elements.length()};
removeAt(it.item.index);
// iterator hasn't changed
return it;
// index hasn't changed; the container pointer shouldn't have changed
// because we shouldn't have detached (detaching happens on obtaining a
// non-const iterator). But just in case we did, reload the pointer.
return { this, qsizetype(it.item.index) };
}
#if QT_STRINGVIEW_LEVEL < 2

71
tests/auto/corelib/serialization/json/tst_qtjson.cpp
View File

@ -1040,14 +1040,37 @@ void tst_QtJson::testObjectIteration()
QCOMPARE(object, object2);
QJsonValue val = *object2.begin();
object2.erase(object2.begin());
auto next = object2.erase(object2.begin());
QCOMPARE(object.size(), 10);
QCOMPARE(object2.size(), 9);
QVERIFY(next == object2.begin());
for (QJsonObject::const_iterator it = object2.constBegin(); it != object2.constEnd(); ++it) {
double d = 1; // we erased the first item
for (auto it = object2.constBegin(); it != object2.constEnd(); ++it, d += 1) {
QJsonValue value = it.value();
QVERIFY(it.value() != val);
QCOMPARE((double)it.key().toInt(), value.toDouble());
QCOMPARE(it.value(), d);
QCOMPARE(it.value().toDouble(), d);
QCOMPARE(it.key().toInt(), value.toDouble());
}
}
{
QJsonObject object2 = object;
QCOMPARE(object, object2);
QJsonValue val = *(object2.end() - 1);
auto next = object2.erase(object2.end() - 1);
QCOMPARE(object.size(), 10);
QCOMPARE(object2.size(), 9);
QVERIFY(next == object2.end());
double d = 0;
for (auto it = object2.constBegin(); it != object2.constEnd(); ++it, d += 1) {
QJsonValue value = it.value();
QVERIFY(it.value() != val);
QCOMPARE(it.value(), d);
QCOMPARE(it.value().toDouble(), d);
QCOMPARE(it.key().toInt(), value.toDouble());
}
}
@ -1057,14 +1080,20 @@ void tst_QtJson::testObjectIteration()
QJsonObject::iterator it = object2.find(QString::number(5));
QJsonValue val = *it;
object2.erase(it);
auto next = object2.erase(it);
QCOMPARE(object.size(), 10);
QCOMPARE(object2.size(), 9);
QCOMPARE(*next, 6);
for (QJsonObject::const_iterator it = object2.constBegin(); it != object2.constEnd(); ++it) {
int i = 0;
for (auto it = object2.constBegin(); it != object2.constEnd(); ++it, ++i) {
if (i == 5)
++i;
QJsonValue value = it.value();
QVERIFY(it.value() != val);
QCOMPARE((double)it.key().toInt(), value.toDouble());
QCOMPARE(it.value(), i);
QCOMPARE(it.value().toInt(), i);
QCOMPARE(it.key().toInt(), value.toDouble());
}
}
@ -1120,14 +1149,38 @@ void tst_QtJson::testArrayIteration()
QCOMPARE(array, array2);
QJsonValue val = *array2.begin();
array2.erase(array2.begin());
auto next = array2.erase(array2.begin());
QCOMPARE(array.size(), 10);
QCOMPARE(array2.size(), 9);
QVERIFY(next == array2.begin());
i = 1;
for (QJsonArray::const_iterator it = array2.constBegin(); it != array2.constEnd(); ++it, ++i) {
for (auto it = array2.constBegin(); it != array2.constEnd(); ++it, ++i) {
QJsonValue value = (*it);
QCOMPARE((double)i, value.toDouble());
QCOMPARE(value.toInt(), i);
QCOMPARE(value.toDouble(), i);
QCOMPARE(it->toInt(), i);
QCOMPARE(it->toDouble(), i);
}
}
{
QJsonArray array2 = array;
QCOMPARE(array, array2);
QJsonValue val = array2.last();
auto next = array2.erase(array2.end() - 1);
QCOMPARE(array.size(), 10);
QCOMPARE(array2.size(), 9);
QVERIFY(next == array2.end());
i = 0;
for (auto it = array2.constBegin(); it != array2.constEnd(); ++it, ++i) {
QJsonValue value = (*it);
QCOMPARE(value.toInt(), i);
QCOMPARE(value.toDouble(), i);
QCOMPARE(it->toInt(), i);
QCOMPARE(it->toDouble(), i);
}
}