From 71fb3633e8d909e9a91e1bee6eaf53c146f25998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Nowacki?= Date: Fri, 14 Feb 2014 16:32:31 +0100 Subject: Unify all mid() functions in QtBase. Up to now, Qt had at least 3 different implementations of the mid(). Only QString::mid implementation was not crashing on edge cases and was protected against overflows, therefore I picked that one as the base implementation, even if it has weird semantics for an invalid input. As a side effect QVector::mid was slightly optimized to not detach in all cases (which follows current QList behavior). Documentation of QVector::mid and QList::mid was updated to not mention "copy of data" which could suggest that the mid() result is detached. QStringRef::mid was fixed and now it follows general Qt behavior, by returning a null value for a null input. Change-Id: Ie9ff5d98372bd193d66508e6dd92b6ed1180ad9b Reviewed-by: Thiago Macieira --- src/corelib/tools/qarraydata.cpp | 29 ++++++++++++++++ src/corelib/tools/qarraydata.h | 8 +++++ src/corelib/tools/qbytearray.cpp | 23 +++++++------ src/corelib/tools/qlist.cpp | 6 ++-- src/corelib/tools/qlist.h | 14 ++++++-- src/corelib/tools/qstring.cpp | 73 ++++++++++++++++++++-------------------- src/corelib/tools/qvector.cpp | 6 ++-- src/corelib/tools/qvector.h | 15 ++++++--- 8 files changed, 114 insertions(+), 60 deletions(-) diff --git a/src/corelib/tools/qarraydata.cpp b/src/corelib/tools/qarraydata.cpp index 2d744f97c0..4047bf611f 100644 --- a/src/corelib/tools/qarraydata.cpp +++ b/src/corelib/tools/qarraydata.cpp @@ -130,4 +130,33 @@ void QArrayData::deallocate(QArrayData *data, size_t objectSize, ::free(data); } +namespace QtPrivate { +/*! + \internal +*/ +QContainerImplHelper::CutResult QContainerImplHelper::mid(int originalLength, int *_position, int *_length) +{ + int &position = *_position; + int &length = *_length; + if (position > originalLength) + return Null; + + if (position < 0) { + if (length < 0 || length + position >= originalLength) + return Full; + if (length + position <= 0) + return Null; + length += position; + position = 0; + } else if (uint(length) > uint(originalLength - position)) { + length = originalLength - position; + } + + if (position == 0 && length == originalLength) + return Full; + + return length > 0 ? Subset : Empty; +} +} + QT_END_NAMESPACE diff --git a/src/corelib/tools/qarraydata.h b/src/corelib/tools/qarraydata.h index 5a8c46b582..90503786e4 100644 --- a/src/corelib/tools/qarraydata.h +++ b/src/corelib/tools/qarraydata.h @@ -361,6 +361,14 @@ namespace QtPrivate { QT_PREPEND_NAMESPACE(QtPrivate::qMakeArrayLiteral)( Array ) #endif // !defined(Q_ARRAY_LITERAL) +namespace QtPrivate { +struct Q_CORE_EXPORT QContainerImplHelper +{ + enum CutResult { Null, Empty, Full, Subset }; + static CutResult mid(int originalLength, int *position, int *length); +}; +} + QT_END_NAMESPACE #endif // include guard diff --git a/src/corelib/tools/qbytearray.cpp b/src/corelib/tools/qbytearray.cpp index 9c2a242e8e..6fed3ae5c9 100644 --- a/src/corelib/tools/qbytearray.cpp +++ b/src/corelib/tools/qbytearray.cpp @@ -2677,19 +2677,22 @@ QByteArray QByteArray::right(int len) const QByteArray QByteArray::mid(int pos, int len) const { - if ((d->size == 0 && d->ref.isStatic()) || pos > d->size) + using namespace QtPrivate; + switch (QContainerImplHelper::mid(size(), &pos, &len)) { + case QContainerImplHelper::Null: return QByteArray(); - if (len < 0) - len = d->size - pos; - if (pos < 0) { - len += pos; - pos = 0; + case QContainerImplHelper::Empty: + { + QByteArrayDataPtr empty = { Data::allocate(0) }; + return QByteArray(empty); } - if (len + pos > d->size) - len = d->size - pos; - if (pos == 0 && len == d->size) + case QContainerImplHelper::Full: return *this; - return QByteArray(d->data() + pos, len); + case QContainerImplHelper::Subset: + return QByteArray(d->data() + pos, len); + } + Q_UNREACHABLE(); + return QByteArray(); } /*! diff --git a/src/corelib/tools/qlist.cpp b/src/corelib/tools/qlist.cpp index 0811c3793e..db49fe9802 100644 --- a/src/corelib/tools/qlist.cpp +++ b/src/corelib/tools/qlist.cpp @@ -491,11 +491,11 @@ void **QListData::erase(void **xi) /*! \fn QList QList::mid(int pos, int length) const - Returns a list whose elements are copied from this list, + Returns a sub-list which includes elements from this list, starting at position \a pos. If \a length is -1 (the default), all - elements from \a pos are copied; otherwise \a length elements (or + elements from \a pos are included; otherwise \a length elements (or all remaining elements if there are less than \a length elements) - are copied. + are included. */ /*! \fn QList::QList() diff --git a/src/corelib/tools/qlist.h b/src/corelib/tools/qlist.h index 9e4ba70908..4c52c22300 100644 --- a/src/corelib/tools/qlist.h +++ b/src/corelib/tools/qlist.h @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -646,10 +647,17 @@ inline void QList::move(int from, int to) template Q_OUTOFLINE_TEMPLATE QList QList::mid(int pos, int alength) const { - if (alength < 0 || pos > size() - alength) - alength = size() - pos; - if (pos == 0 && alength == size()) + using namespace QtPrivate; + switch (QContainerImplHelper::mid(size(), &pos, &alength)) { + case QContainerImplHelper::Null: + case QContainerImplHelper::Empty: + return QList(); + case QContainerImplHelper::Full: return *this; + case QContainerImplHelper::Subset: + break; + } + QList cpy; if (alength <= 0) return cpy; diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index aac9c493c3..541a853487 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -4090,21 +4090,22 @@ QString QString::right(int n) const QString QString::mid(int position, int n) const { - if (position > d->size) + using namespace QtPrivate; + switch (QContainerImplHelper::mid(d->size, &position, &n)) { + case QContainerImplHelper::Null: return QString(); - if (position < 0) { - if (n < 0 || n + position >= d->size) - return *this; - if (n + position <= 0) - return QString(); - - n += position; - position = 0; - } else if (uint(n) > uint(d->size - position)) - n = d->size - position; - if (position == 0 && n == d->size) + case QContainerImplHelper::Empty: + { + QStringDataPtr empty = { Data::allocate(0) }; + return QString(empty); + } + case QContainerImplHelper::Full: return *this; - return QString((const QChar*) d->data() + position, n); + case QContainerImplHelper::Subset: + return QString((const QChar*)d->data() + position, n); + } + Q_UNREACHABLE(); + return QString(); } /*! @@ -8794,19 +8795,19 @@ QStringRef QString::rightRef(int n) const */ QStringRef QStringRef::mid(int pos, int n) const { - if (pos > m_size) + using namespace QtPrivate; + switch (QContainerImplHelper::mid(m_size, &pos, &n)) { + case QContainerImplHelper::Null: return QStringRef(); - if (pos < 0) { - if (n < 0 || n + pos >= m_size) - return QStringRef(m_string, m_position, m_size); - if (n + pos <= 0) - return QStringRef(); - n += pos; - pos = 0; - } else if (uint(n) > uint(m_size - pos)) { - n = m_size - pos; + case QContainerImplHelper::Empty: + return QStringRef(m_string, 0, 0); + case QContainerImplHelper::Full: + return *this; + case QContainerImplHelper::Subset: + return QStringRef(m_string, pos + m_position, n); } - return QStringRef(m_string, pos + m_position, n); + Q_UNREACHABLE(); + return QStringRef(); } /*! @@ -8831,19 +8832,19 @@ QStringRef QStringRef::mid(int pos, int n) const */ QStringRef QString::midRef(int position, int n) const { - if (position > d->size) + using namespace QtPrivate; + switch (QContainerImplHelper::mid(d->size, &position, &n)) { + case QContainerImplHelper::Null: return QStringRef(); - if (position < 0) { - if (n < 0 || n + position >= d->size) - return QStringRef(this, 0, d->size); - if (n + position <= 0) - return QStringRef(); - - n += position; - position = 0; - } else if (uint(n) > uint(d->size - position)) - n = d->size - position; - return QStringRef(this, position, n); + case QContainerImplHelper::Empty: + return QStringRef(this, 0, 0); + case QContainerImplHelper::Full: + return QStringRef(this, 0, d->size); + case QContainerImplHelper::Subset: + return QStringRef(this, position, n); + } + Q_UNREACHABLE(); + return QStringRef(); } /*! diff --git a/src/corelib/tools/qvector.cpp b/src/corelib/tools/qvector.cpp index b2b4315545..e1a451f953 100644 --- a/src/corelib/tools/qvector.cpp +++ b/src/corelib/tools/qvector.cpp @@ -180,11 +180,11 @@ /*! \fn QVector QVector::mid(int pos, int length = -1) const - Returns a vector whose elements are copied from this vector, + Returns a sub-vector which contains elements from this vector, starting at position \a pos. If \a length is -1 (the default), all - elements after \a pos are copied; otherwise \a length elements (or + elements after \a pos are included; otherwise \a length elements (or all remaining elements if there are less than \a length elements) - are copied. + are included. */ diff --git a/src/corelib/tools/qvector.h b/src/corelib/tools/qvector.h index 0a32c96958..9692477e18 100644 --- a/src/corelib/tools/qvector.h +++ b/src/corelib/tools/qvector.h @@ -822,12 +822,17 @@ int QVector::count(const T &t) const template Q_OUTOFLINE_TEMPLATE QVector QVector::mid(int pos, int len) const { - if (len < 0) - len = size() - pos; - if (pos == 0 && len == size()) + using namespace QtPrivate; + switch (QContainerImplHelper::mid(d->size, &pos, &len)) { + case QContainerImplHelper::Null: + case QContainerImplHelper::Empty: + return QVector(); + case QContainerImplHelper::Full: return *this; - if (pos + len > size()) - len = size() - pos; + case QContainerImplHelper::Subset: + break; + } + QVector copy; copy.reserve(len); for (int i = pos; i < pos + len; ++i) -- cgit v1.2.3