QCborMap: fix assigning elements from the map to itself

Similar to the QJsonObject issue of the previous commit (found with the
same tests, but not the same root cause). One fix was that copying of
byte data from the QByteArray to itself won't work if the array
reallocates. The second was that

  assign(*that, other.concrete());

fails to set other.d to null after moving. By calling the operator=, we
get the proper sequence of events.

[ChangeLog][QtCore][QCborMap] Fixed some issues relating to assigning
elements from a map to itself.

Note: QCborMap is not affected by the design flaw discovered in
QJsonObject because it always appends elements (it's unsorted), so
existing QCborValueRef references still refer to the same value.

Task-number: QTBUG-83366
Change-Id: Ibdc95e9af7bd456a94ecfffd1603df846f46094d
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Thiago Macieira 2020-04-08 11:47:33 -03:00
parent ddc7b3c156
commit 57a57fda78
2 changed files with 78 additions and 3 deletions

View File

@ -994,8 +994,12 @@ void QCborContainerPrivate::replaceAt_complex(Element &e, const QCborValue &valu
e = value.container->elements.at(value.n);
// Copy string data, if any
if (const ByteData *b = value.container->byteData(value.n))
if (const ByteData *b = value.container->byteData(value.n)) {
if (this == value.container)
e.value = addByteData(b->toByteArray(), b->len);
else
e.value = addByteData(b->byte(), b->len);
}
if (disp == MoveContainer)
value.container->deref();
@ -2649,7 +2653,7 @@ void QCborValueRef::assign(QCborValueRef that, QCborValue &&other)
void QCborValueRef::assign(QCborValueRef that, const QCborValueRef other)
{
// ### optimize?
assign(that, other.concrete());
that = other.concrete();
}
QCborValue QCborValueRef::concrete(QCborValueRef self) noexcept

View File

@ -79,6 +79,7 @@ private slots:
void mapEmptyDetach();
void mapSimpleInitializerList();
void mapMutation();
void mapMutateWithCopies();
void mapStringValues();
void mapStringKeys();
void mapInsertRemove_data() { basics_data(); }
@ -923,6 +924,76 @@ void tst_QCborValue::mapMutation()
QCOMPARE(val[any][3].toMap().size(), 1);
}
void tst_QCborValue::mapMutateWithCopies()
{
{
QCborMap map;
map[QLatin1String("prop1")] = "TEST";
QCOMPARE(map.size(), 1);
QCOMPARE(map.value("prop1"), "TEST");
map[QLatin1String("prop2")] = map.value("prop1");
QCOMPARE(map.size(), 2);
QCOMPARE(map.value("prop1"), "TEST");
QCOMPARE(map.value("prop2"), "TEST");
}
{
// see QTBUG-83366
QCborMap map;
map[QLatin1String("value")] = "TEST";
QCOMPARE(map.size(), 1);
QCOMPARE(map.value("value"), "TEST");
QCborValue v = map.value("value");
map[QLatin1String("prop2")] = v;
QCOMPARE(map.size(), 2);
QCOMPARE(map.value("value"), "TEST");
QCOMPARE(map.value("prop2"), "TEST");
}
{
QCborMap map;
map[QLatin1String("value")] = "TEST";
QCOMPARE(map.size(), 1);
QCOMPARE(map.value("value"), "TEST");
// same as previous, but this is a QJsonValueRef
QCborValueRef rv = map[QLatin1String("prop2")];
rv = map[QLatin1String("value")];
QCOMPARE(map.size(), 2);
QCOMPARE(map.value("value"), "TEST");
QCOMPARE(map.value("prop2"), "TEST");
}
{
QCborMap map;
map[QLatin1String("value")] = "TEST";
QCOMPARE(map.size(), 1);
QCOMPARE(map.value("value"), "TEST");
// same as previous, but now we call the operator[] that reallocates
// after we create the source QCborValueRef
QCborValueRef rv = map[QLatin1String("value")];
map[QLatin1String("prop2")] = rv;
QCOMPARE(map.size(), 2);
QCOMPARE(map.value("value"), "TEST");
QCOMPARE(map.value("prop2"), "TEST");
}
{
QCborMap map;
map[QLatin1String("value")] = "TEST";
QCOMPARE(map.size(), 1);
QCOMPARE(map.value("value"), "TEST");
QCborValueRef v = map[QLatin1String("value")];
QCborMap map2 = map;
map.insert(QLatin1String("prop2"), v);
QCOMPARE(map.size(), 2);
QCOMPARE(map.value("value"), "TEST");
QCOMPARE(map.value("prop2"), "TEST");
QCOMPARE(map2.size(), 1);
QCOMPARE(map2.value("value"), "TEST");
}
}
void tst_QCborValue::arrayPrepend()
{
QCborArray a;