Implement QCborContainerPrivate::compact()

... and use it in QCborContainerPrivate::replaceAt_complex() to avoid
unconstrained memory growth in certain scenarios.

Remove the `reserved` parameter, because it was referring to the
elements array, not to the byte data, so it cannot really be used
in the implementation.

Fixes: QTBUG-109073
Change-Id: I2e8fe7e4a4bf7a0ce06c87ca657f2bc01bae0341
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 83f0796192d9f0ed3bc005dbcf68b98de62955b8)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Ivan Solovev 2024-02-01 17:47:52 +01:00 committed by Qt Cherry-pick Bot
parent f0bc5cac4f
commit 6e269fe18e
2 changed files with 43 additions and 14 deletions

View File

@ -922,14 +922,24 @@ QCborContainerPrivate::~QCborContainerPrivate()
}
}
void QCborContainerPrivate::compact(qsizetype reserved)
void QCborContainerPrivate::compact()
{
if (usedData > data.size() / 2)
return;
// 50% savings if we recreate the byte data
// ### TBD
Q_UNUSED(reserved);
QByteArray newData;
QByteArray::size_type newUsedData = 0;
// Compact only elements that have byte data.
// Nested containers will be compacted when their data changes.
for (auto &e : elements) {
if (e.flags & Element::HasByteData) {
if (const ByteData *b = byteData(e))
e.value = addByteDataImpl(newData, newUsedData, b->byte(), b->len);
}
}
data = newData;
usedData = newUsedData;
}
QCborContainerPrivate *QCborContainerPrivate::clone(QCborContainerPrivate *d, qsizetype reserved)
@ -941,7 +951,7 @@ QCborContainerPrivate *QCborContainerPrivate::clone(QCborContainerPrivate *d, qs
QExplicitlySharedDataPointer u(new QCborContainerPrivate(*d));
if (reserved >= 0) {
u->elements.reserve(reserved);
u->compact(reserved);
u->compact();
}
d = u.take();
@ -1013,11 +1023,24 @@ void QCborContainerPrivate::replaceAt_complex(Element &e, const QCborValue &valu
// Copy string data, if any
if (const ByteData *b = value.container->byteData(value.n)) {
if (this == value.container)
e.value = addByteData(b->toByteArray(), b->len);
else
const auto flags = e.flags;
// The element e has an invalid e.value, because it is copied from
// value. It means that calling compact() will trigger an assertion
// or just silently corrupt the data.
// Temporarily unset the Element::HasByteData flag in order to skip
// the element e in the call to compact().
e.flags = e.flags & ~Element::HasByteData;
if (this == value.container) {
const QByteArray valueData = b->toByteArray();
compact();
e.value = addByteData(valueData, valueData.size());
} else {
compact();
e.value = addByteData(b->byte(), b->len);
}
// restore the flags
e.flags = flags;
}
if (disp == MoveContainer)
value.container->deref();
@ -1053,7 +1076,7 @@ QCborValue QCborContainerPrivate::extractAt_complex(Element e)
// make a shallow copy of the byte data
container->appendByteData(b->byte(), b->len, e.type, e.flags);
usedData -= b->len + qsizetype(sizeof(*b));
compact(elements.size());
compact();
} else {
// just share with the original byte data
container->data = data;

View File

@ -103,18 +103,19 @@ public:
QList<QtCbor::Element> elements;
void deref() { if (!ref.deref()) delete this; }
void compact(qsizetype reserved);
void compact();
static QCborContainerPrivate *clone(QCborContainerPrivate *d, qsizetype reserved = -1);
static QCborContainerPrivate *detach(QCborContainerPrivate *d, qsizetype reserved);
static QCborContainerPrivate *grow(QCborContainerPrivate *d, qsizetype index);
qptrdiff addByteData(const char *block, qsizetype len)
static qptrdiff addByteDataImpl(QByteArray &target, QByteArray::size_type &targetUsed,
const char *block, qsizetype len)
{
// This function does not do overflow checking, since the len parameter
// is expected to be trusted. There's another version of this function
// in decodeStringFromCbor(), which checks.
qptrdiff offset = data.size();
qptrdiff offset = target.size();
// align offset
offset += alignof(QtCbor::ByteData) - 1;
@ -122,10 +123,10 @@ public:
qptrdiff increment = qptrdiff(sizeof(QtCbor::ByteData)) + len;
usedData += increment;
data.resize(offset + increment);
targetUsed += increment;
target.resize(offset + increment);
char *ptr = data.begin() + offset;
char *ptr = target.begin() + offset;
auto b = new (ptr) QtCbor::ByteData;
b->len = len;
if (block)
@ -134,6 +135,11 @@ public:
return offset;
}
qptrdiff addByteData(const char *block, qsizetype len)
{
return addByteDataImpl(data, usedData, block, len);
}
const QtCbor::ByteData *byteData(QtCbor::Element e) const
{
if ((e.flags & QtCbor::Element::HasByteData) == 0)