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 <thiago.macieira@intel.com>

1 files changed, 6 insertions, 5 deletions

diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h
index e2cb7fbf23..b75c2975b6 100644
--- a/src/corelib/tools/qvector.h
+++ b/src/corelib/tools/qvector.h
@@ -581,14 +581,15 @@ typename QVector<T>::iterator QVector<T>::insert(iterator before, size_type n, c
 template <typename T>
 typename QVector<T>::iterator QVector<T>::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.