From 796f85b611da5d689e08398e027b130824720a23 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jo=C3=A3o=20Abecasis?= Date: Fri, 8 Jun 2012 15:25:26 +0200 Subject: [PATCH] Don't operate on bogus data, assert on preconditions instead QVector::erase shouldn't try to make sense of iterators it doesn't own, so the validation being done here is bogus and dangerous. Instead, it's preferrable to assert, the user needs to ensure proper ownership. The case of erasing an empty sequence is not checked for preconditions to allow QVector v; v.erase(v.begin(), v.end()); , while being stricter on other uses. Autotests were using ill-formed calls to the single argument erase() function on an empty vector and were fixed. This function erases exactly one element, the one pointed to by abegin and require the element exist and be valid. Change-Id: I5f1a6d0d8da072eae0c73a3012620c4ce1065cf0 Reviewed-by: Thiago Macieira --- src/corelib/tools/qvector.h | 11 ++++++----- tests/auto/corelib/tools/qvector/tst_qvector.cpp | 8 -------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h index e2cb7fb..b75c297 100644 --- a/src/corelib/tools/qvector.h +++ b/src/corelib/tools/qvector.h @@ -581,14 +581,15 @@ typename QVector::iterator QVector::insert(iterator before, size_type n, c template typename QVector::iterator QVector::erase(iterator abegin, iterator aend) { - if (abegin < d->begin()) - abegin = d->begin(); - if (aend > d->end()) - aend = d->end(); + const int itemsToErase = aend - abegin; + + if (!itemsToErase) + return abegin; + Q_ASSERT(abegin >= d->begin()); + Q_ASSERT(aend <= d->end()); Q_ASSERT(abegin <= aend); - const int itemsToErase = aend - abegin; const int itemsUntouched = abegin - d->begin(); // FIXME we could do a proper realloc, which copy constructs only needed data. diff --git a/tests/auto/corelib/tools/qvector/tst_qvector.cpp b/tests/auto/corelib/tools/qvector/tst_qvector.cpp index 56570b8..09d3051 100644 --- a/tests/auto/corelib/tools/qvector/tst_qvector.cpp +++ b/tests/auto/corelib/tools/qvector/tst_qvector.cpp @@ -796,16 +796,12 @@ void tst_QVector::eraseEmpty() const { { QVector v; - v.erase(v.begin()); - QCOMPARE(v.size(), 0); v.erase(v.begin(), v.end()); QCOMPARE(v.size(), 0); } { QVector v; v.setSharable(false); - v.erase(v.begin()); - QCOMPARE(v.size(), 0); v.erase(v.begin(), v.end()); QCOMPARE(v.size(), 0); } @@ -836,8 +832,6 @@ void tst_QVector::eraseEmptyReserved() const { QVector v; v.reserve(10); - v.erase(v.begin()); - QCOMPARE(v.size(), 0); v.erase(v.begin(), v.end()); QCOMPARE(v.size(), 0); } @@ -845,8 +839,6 @@ void tst_QVector::eraseEmptyReserved() const QVector v; v.reserve(10); v.setSharable(false); - v.erase(v.begin()); - QCOMPARE(v.size(), 0); v.erase(v.begin(), v.end()); QCOMPARE(v.size(), 0); } -- 2.7.4