QMovableArrayOps::Inserter: move the displace() call into the ctor

Coverity complained that ~Inserter() will access at least displaceFrom
and displaceTo uninitialized, which is correct, if none of the
insert() overloads have been called in-between. This is a brittle
construct, even though, currently, all users of the class comply.

To help Coverity (and other readers of the code) understand what's
going on, move the displace() call that creates the hole in the
container which the insert() overloads then fill from said overloads
into the ctor, original-RAII style (ctor acquires the hole, dtor closes
it, if needed). This means all fields are initialized in the ctor now.

This is safe, as displace() cannot fail by itself (just a memmove()),
but requires moving the (pos, n) (= hole) information into the ctor
instead. The displace() call in the insert() overloads now becomes a
read of its return argument, displaceFrom.

(Incidentally, that shows that we have maintained the same pointer
twice in the insert overloads, something we'll clean up in a
follow-up.)

In order to verify the insert() post-condition (ie. that we filled the
whole hole created by the ctor, or threw an exception trying),
continue to pass the count to insert().

As a drive-by, rename the insert()-overloads to insertRange() and
insertFill() to match the existing insertOne() function, and to better
express what their purpose is.

Pick-to: 6.8 6.5
Coverity-Id: 378364
Coverity-Id: 378461
Coverity-Id: 378343
Change-Id: I1a6bc1ea0e5506d473f6089818797b02d09ba13e
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
(cherry picked from commit aa8e8ffd321dc96650c11ebe3cd4017e7cc8edac)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2025-03-04 14:37:20 +01:00 committed by Qt Cherry-pick Bot
parent e37e3d9045
commit b173da88f4

View File

@ -640,7 +640,12 @@ public:
qsizetype nInserts = 0;
qsizetype bytes;
Inserter(QArrayDataPointer<T> *d) : data(d) { }
void verifyPost(T *where)
{ Q_ASSERT(where == displaceTo); }
explicit Inserter(QArrayDataPointer<T> *d, qsizetype pos, qsizetype n)
: data(d)
{ displace(pos, n); }
~Inserter() {
if constexpr (!std::is_nothrow_copy_constructible_v<T>) {
if (displaceFrom != displaceTo) {
@ -664,9 +669,9 @@ public:
return insertionPoint;
}
void insert(qsizetype pos, const T *source, qsizetype n)
void insertRange(const T *source, qsizetype n)
{
T *where = displace(pos, n);
T *where = displaceFrom;
while (n--) {
new (where) T(*source);
@ -674,25 +679,27 @@ public:
++source;
++displaceFrom;
}
verifyPost(where);
}
void insert(qsizetype pos, const T &t, qsizetype n)
void insertFill(const T &t, qsizetype n)
{
T *where = displace(pos, n);
T *where = displaceFrom;
while (n--) {
new (where) T(t);
++where;
++displaceFrom;
}
verifyPost(where);
}
void insertOne(qsizetype pos, T &&t)
void insertOne(T &&t)
{
T *where = displace(pos, 1);
T *where = displaceFrom;
new (where) T(std::move(t));
++displaceFrom;
Q_ASSERT(displaceFrom == displaceTo);
verifyPost(++where);
}
};
@ -718,7 +725,7 @@ public:
++this->size;
}
} else {
Inserter(this).insert(i, data, n);
Inserter(this, i, n).insertRange(data, n);
}
}
@ -742,7 +749,7 @@ public:
++this->size;
}
} else {
Inserter(this).insert(i, copy, n);
Inserter(this, i, n).insertFill(copy, n);
}
}
@ -774,7 +781,7 @@ public:
--this->ptr;
++this->size;
} else {
Inserter(this).insertOne(i, std::move(tmp));
Inserter(this, i, 1).insertOne(std::move(tmp));
}
}