1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ensure left/right/mid behave in a compatible way

Browse Source 
QString and QStringRef did bounds checking for left/right/mid, whereas
QStringView was asserting on out of bounds.

Relax the behavior for QStringView and do bounds checking on pos/n
as well. This removes a source of potentially hidden errors when porting
from QStringRef (or QString) to QStringView.

Unfortunately, one difference remains, where QByteArray::left/right()
behaves differently (and somewhat more sane) than QString and
QStringRef. We're keeping the difference here, as it has been around
for many years.

Mark left/right/mid as obsolete and to be replaced with the new
first/last/slice methods.

Change-Id: I18c203799ba78c928a4610a6038089f27696c22e
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Lars Knoll 2020-06-02 15:51:15 +02:00
parent fd856532d7
commit d2833a3ce5
9 changed files with 235 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/corelib/text/qbytearray.cpp
View File

@ -2914,13 +2914,17 @@ bool QByteArray::isLower() const
Returns a byte array that contains the first \a len bytes of this byte
array.
\obsolete Use first() instead in new code.
The entire byte array is returned if \a len is greater than
size().
Returns an empty QByteArray if \a len is smaller than 0.
Example:
\snippet code/src_corelib_text_qbytearray.cpp 27
\sa startsWith(), right(), mid(), chopped(), chop(), truncate()
\sa first(), last(), startsWith(), chopped(), chop(), truncate()
*/
QByteArray QByteArray::left(int len) const
@ -2935,15 +2939,18 @@ QByteArray QByteArray::left(int len) const
/*!
Returns a byte array that contains the last \a len bytes of this byte array.
\obsolete Use last() instead in new code.
The entire byte array is returned if \a len is greater than
size().
Returns an empty QByteArray if \a len is smaller than 0.
Example:
\snippet code/src_corelib_text_qbytearray.cpp 28
\sa endsWith(), left(), mid(), chopped(), chop(), truncate()
\sa endsWith(), last(), first(), slice(), chopped(), chop(), truncate()
*/
QByteArray QByteArray::right(int len) const
{
if (len >= size())
@ -2957,6 +2964,8 @@ QByteArray QByteArray::right(int len) const
Returns a byte array containing \a len bytes from this byte array,
starting at position \a pos.
\obsolete Use slice() instead in new code.
If \a len is -1 (the default), or \a pos + \a len >= size(),
returns a byte array containing all bytes starting at position \a
pos until the end of the byte array.
@ -2964,13 +2973,15 @@ QByteArray QByteArray::right(int len) const
Example:
\snippet code/src_corelib_text_qbytearray.cpp 29
\sa left(), right(), chopped(), chop(), truncate()
\sa first(), last(), slice(), chopped(), chop(), truncate()
*/
QByteArray QByteArray::mid(int pos, int len) const
{
qsizetype p = pos;
qsizetype l = len;
using namespace QtPrivate;
switch (QContainerImplHelper::mid(size(), &pos, &len)) {
switch (QContainerImplHelper::mid(size(), &p, &l)) {
case QContainerImplHelper::Null:
return QByteArray();
case QContainerImplHelper::Empty:
@ -2982,7 +2993,7 @@ QByteArray QByteArray::mid(int pos, int len) const
case QContainerImplHelper::Full:
return *this;
case QContainerImplHelper::Subset:
return QByteArray(d.data() + pos, len);
return QByteArray(d.data() + p, l);
}
Q_UNREACHABLE();
return QByteArray();

53
src/corelib/text/qstring.cpp
View File

@ -4530,12 +4530,14 @@ QString QString::section(const QRegularExpression &re, int start, int end, Secti
Returns a substring that contains the \a n leftmost characters
of the string.
\obsolete Use first() instead in new code.
The entire string is returned if \a n is greater than or equal
to size(), or less than zero.
\snippet qstring/main.cpp 31
\sa right(), mid(), startsWith(), chopped(), chop(), truncate()
\sa first(), last(), startsWith(), chopped(), chop(), truncate()
*/
QString QString::left(int n) const
{
@ -4548,12 +4550,14 @@ QString QString::left(int n) const
Returns a substring that contains the \a n rightmost characters
of the string.
\obsolete Use last() instead in new code.
The entire string is returned if \a n is greater than or equal
to size(), or less than zero.
\snippet qstring/main.cpp 48
\sa left(), mid(), endsWith(), chopped(), chop(), truncate()
\sa endsWith(), last(), first(), slice(), chopped(), chop(), truncate()
*/
QString QString::right(int n) const
{
@ -4566,6 +4570,8 @@ QString QString::right(int n) const
Returns a string that contains \a n characters of this string,
starting at the specified \a position index.
\obsolete Use slice() instead in new code.
Returns a null string if the \a position index exceeds the
length of the string. If there are less than \a n characters
available in the string starting at the given \a position, or if
@ -4576,13 +4582,15 @@ QString QString::right(int n) const
\snippet qstring/main.cpp 34
\sa left(), right(), chopped(), chop(), truncate()
\sa first(), last(), slice(), chopped(), chop(), truncate()
*/
QString QString::mid(int position, int n) const
{
qsizetype p = position;
qsizetype l = n;
using namespace QtPrivate;
switch (QContainerImplHelper::mid(size(), &position, &n)) {
switch (QContainerImplHelper::mid(size(), &p, &l)) {
case QContainerImplHelper::Null:
return QString();
case QContainerImplHelper::Empty:
@ -4594,7 +4602,7 @@ QString QString::mid(int position, int n) const
case QContainerImplHelper::Full:
return *this;
case QContainerImplHelper::Subset:
return QString(constData() + position, n);
return QString(constData() + p, l);
}
Q_UNREACHABLE();
return QString();
@ -9375,28 +9383,17 @@ QString &QString::setRawData(const QChar *unicode, int size)
\sa crbegin(), rend(), cend()
*/
/*! \fn QLatin1String QLatin1String::mid(int start) const
\since 5.8
Returns the substring starting at position \a start in this object,
and extending to the end of the string.
\note This function performs no error checking.
The behavior is undefined when \a start < 0 or \a start > size().
\sa left(), right(), chopped(), chop(), truncate()
*/
/*! \fn QLatin1String QLatin1String::mid(int start, int length) const
\since 5.8
\overload
Returns the substring of length \a length starting at position
\a start in this object.
\note This function performs no error checking.
The behavior is undefined when \a start < 0, \a length < 0,
or \a start + \a length > size().
Returns a null string if the \a start index exceeds the
length of the string. If there are less than \a length characters
available in the string starting at \a start, or if
\a length is negative (default), the function returns all characters
that are available from \a start.
\sa left(), right(), chopped(), chop(), truncate()
*/
@ -9407,8 +9404,8 @@ QString &QString::setRawData(const QChar *unicode, int size)
Returns the substring of length \a length starting at position
0 in this object.
\note This function performs no error checking.
The behavior is undefined when \a length < 0 or \a length > size().
The entire string is returned if \a length is greater than or equal
to size(), or less than zero.
\sa mid(), right(), chopped(), chop(), truncate()
*/
@ -9419,8 +9416,8 @@ QString &QString::setRawData(const QChar *unicode, int size)
Returns the substring of length \a length starting at position
size() - \a length in this object.
\note This function performs no error checking.
The behavior is undefined when \a length < 0 or \a length > size().
The entire string is returned if \a length is greater than or equal
to size(), or less than zero.
\sa mid(), left(), chopped(), chop(), truncate()
*/
@ -10777,8 +10774,10 @@ QStringRef QString::rightRef(int n) const
*/
QStringRef QStringRef::mid(int pos, int n) const
{
qsizetype p = pos;
qsizetype l = n;
using namespace QtPrivate;
switch (QContainerImplHelper::mid(m_size, &pos, &n)) {
switch (QContainerImplHelper::mid(m_size, &p, &l)) {
case QContainerImplHelper::Null:
return QStringRef();
case QContainerImplHelper::Empty:
@ -10786,7 +10785,7 @@ QStringRef QStringRef::mid(int pos, int n) const
case QContainerImplHelper::Full:
return *this;
case QContainerImplHelper::Subset:
return QStringRef(m_string, pos + m_position, n);
return QStringRef(m_string, p + m_position, l);
}
Q_UNREACHABLE();
return QStringRef();

28
src/corelib/text/qstring.h
View File

@ -179,14 +179,26 @@ public:
const_reverse_iterator rend() const noexcept { return const_reverse_iterator(begin()); }
const_reverse_iterator crend() const noexcept { return const_reverse_iterator(begin()); }
Q_DECL_CONSTEXPR QLatin1String mid(int pos) const
{ return Q_ASSERT(pos >= 0), Q_ASSERT(pos <= size()), QLatin1String(m_data + pos, m_size - pos); }
Q_DECL_CONSTEXPR QLatin1String mid(int pos, int n) const
{ return Q_ASSERT(pos >= 0), Q_ASSERT(n >= 0), Q_ASSERT(pos + n <= size()), QLatin1String(m_data + pos, n); }
Q_DECL_CONSTEXPR QLatin1String left(int n) const
{ return Q_ASSERT(n >= 0), Q_ASSERT(n <= size()), QLatin1String(m_data, n); }
Q_DECL_CONSTEXPR QLatin1String right(int n) const
{ return Q_ASSERT(n >= 0), Q_ASSERT(n <= size()), QLatin1String(m_data + m_size - n, n); }
constexpr QLatin1String mid(int pos, int n = -1) const
{
qsizetype p = pos;
qsizetype l = n;
using namespace QtPrivate;
auto result = QContainerImplHelper::mid(size(), &p, &l);
return result == QContainerImplHelper::Null ? QLatin1String() : QLatin1String(m_data + p, l);
}
constexpr QLatin1String left(int n) const
{
if (size_t(n) >= size_t(size()))
n = size();
return QLatin1String(m_data, n);
}
constexpr QLatin1String right(int n) const
{
if (size_t(n) >= size_t(size()))
n = size();
return QLatin1String(m_data + m_size - n, n);
}
Q_REQUIRED_RESULT Q_DECL_CONSTEXPR QLatin1String chopped(int n) const
{ return Q_ASSERT(n >= 0), Q_ASSERT(n <= size()), QLatin1String(m_data, m_size - n); }

37
src/corelib/text/qstringview.cpp
View File

@ -610,50 +610,49 @@ QT_BEGIN_NAMESPACE
\sa back(), front(), first()
*/
/*!
\fn QStringView QStringView::mid(qsizetype start) const
Returns the substring starting at position \a start in this object,
and extending to the end of the string.
\note The behavior is undefined when \a start < 0 or \a start > size().
\sa left(), right(), chopped(), chop(), truncate()
*/
/*!
\fn QStringView QStringView::mid(qsizetype start, qsizetype length) const
\overload
Returns the substring of length \a length starting at position
\a start in this object.
\note The behavior is undefined when \a start < 0, \a length < 0,
or \a start + \a length > size().
\obsolete Use slice() instead in new code.
\sa left(), right(), chopped(), chop(), truncate()
Returns an empty string view if \a start exceeds the
length of the string. If there are less than \a length characters
available in the string starting at \a start, or if
\a length is negative (default), the function returns all characters that
are available from \a start.
\sa first(), last(), slice(), chopped(), chop(), truncate()
*/
/*!
\fn QStringView QStringView::left(qsizetype length) const
\obsolete Use first() instead in new code.
Returns the substring of length \a length starting at position
0 in this object.
\note The behavior is undefined when \a length < 0 or \a length > size().
The entire string is returned if \a length is greater than or equal
to size(), or less than zero.
\sa mid(), right(), chopped(), chop(), truncate()
\sa first(), last(), slice(), startsWith(), chopped(), chop(), truncate()
*/
/*!
\fn QStringView QStringView::right(qsizetype length) const
\obsolete Use last() instead in new code.
Returns the substring of length \a length starting at position
size() - \a length in this object.
\note The behavior is undefined when \a length < 0 or \a length > size().
The entire string is returned if \a length is greater than or equal
to size(), or less than zero.
\sa mid(), left(), chopped(), chop(), truncate()
\sa first(), last(), slice(), endsWith(), chopped(), chop(), truncate()
*/
/*!

26
src/corelib/text/qstringview.h
View File

@ -257,14 +257,24 @@ public:
Q_REQUIRED_RESULT Q_DECL_CONSTEXPR QChar at(qsizetype n) const { return (*this)[n]; }
Q_REQUIRED_RESULT Q_DECL_CONSTEXPR QStringView mid(qsizetype pos) const
{ return Q_ASSERT(pos >= 0), Q_ASSERT(pos <= size()), QStringView(m_data + pos, m_size - pos); }
Q_REQUIRED_RESULT Q_DECL_CONSTEXPR QStringView mid(qsizetype pos, qsizetype n) const
{ return Q_ASSERT(pos >= 0), Q_ASSERT(n >= 0), Q_ASSERT(pos + n <= size()), QStringView(m_data + pos, n); }
Q_REQUIRED_RESULT Q_DECL_CONSTEXPR QStringView left(qsizetype n) const
{ return Q_ASSERT(n >= 0), Q_ASSERT(n <= size()), QStringView(m_data, n); }
Q_REQUIRED_RESULT Q_DECL_CONSTEXPR QStringView right(qsizetype n) const
{ return Q_ASSERT(n >= 0), Q_ASSERT(n <= size()), QStringView(m_data + m_size - n, n); }
Q_REQUIRED_RESULT constexpr QStringView mid(qsizetype pos, qsizetype n = -1) const
{
using namespace QtPrivate;
auto result = QContainerImplHelper::mid(size(), &pos, &n);
return result == QContainerImplHelper::Null ? QStringView() : QStringView(m_data + pos, n);
}
Q_REQUIRED_RESULT constexpr QStringView left(qsizetype n) const
{
if (size_t(n) >= size_t(size()))
n = size();
return QStringView(m_data, n);
}
Q_REQUIRED_RESULT constexpr QStringView right(qsizetype n) const
{
if (size_t(n) >= size_t(size()))
n = size();
return QStringView(m_data + m_size - n, n);
}
Q_REQUIRED_RESULT constexpr QStringView first(qsizetype n) const
{ Q_ASSERT(n >= 0); Q_ASSERT(n <= size()); return QStringView(m_data, int(n)); }

29
src/corelib/tools/qarraydata.cpp
View File

@ -281,33 +281,4 @@ 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

32
src/corelib/tools/qarraydata.h
View File

@ -306,7 +306,37 @@ namespace QtPrivate {
struct Q_CORE_EXPORT QContainerImplHelper
{
enum CutResult { Null, Empty, Full, Subset };
static CutResult mid(int originalLength, int *position, int *length);
static constexpr CutResult mid(qsizetype originalLength, qsizetype *_position, qsizetype *_length)
{
qsizetype &position = *_position;
qsizetype &length = *_length;
if (position > originalLength) {
position = 0;
length = 0;
return Null;
}
if (position < 0) {
if (length < 0 || length + position >= originalLength) {
position = 0;
length = originalLength;
return Full;
}
if (length + position <= 0) {
position = length = 0;
return Null;
}
length += position;
position = 0;
} else if (size_t(length) > size_t(originalLength - position)) {
length = originalLength - position;
}
if (position == 0 && length == originalLength)
return Full;
return length > 0 ? Subset : Empty;
}
};
}

8
src/corelib/tools/qvector.h
View File

@ -710,8 +710,10 @@ int QVector<T>::lastIndexOf(const T &t, int from) const noexcept
template <typename T>
inline QVector<T> QVector<T>::mid(int pos, int len) const
{
qsizetype p = pos;
qsizetype l = len;
using namespace QtPrivate;
switch (QContainerImplHelper::mid(d.size, &pos, &len)) {
switch (QContainerImplHelper::mid(d.size, &p, &l)) {
case QContainerImplHelper::Null:
case QContainerImplHelper::Empty:
return QVector();
@ -722,8 +724,8 @@ inline QVector<T> QVector<T>::mid(int pos, int len) const
}
// Allocate memory
DataPointer copied(Data::allocate(len));
copied->copyAppend(constBegin() + pos, constBegin() + pos + len);
DataPointer copied(Data::allocate(l));
copied->copyAppend(constBegin() + p, constBegin() + p + l);
return copied;
}

124
tests/auto/corelib/text/qstringapisymmetry/tst_qstringapisymmetry.cpp
View File

@ -644,9 +644,6 @@ private:
void chop_data();
template <typename String> void chop_impl();
void truncate_data() { left_data(); }
template <typename String> void truncate_impl();
private Q_SLOTS:
void mid_QString_data() { mid_data(); }
@ -660,16 +657,16 @@ private Q_SLOTS:
void mid_QByteArray_data() { mid_data(); }
void mid_QByteArray() { mid_impl<QByteArray>(); }
void left_truncate_QString_data() { left_data(); }
void left_truncate_QString() { left_impl<QString>(); }
void left_truncate_QStringRef_data() { left_data(); }
void left_truncate_QStringRef() { left_impl<QStringRef>(); }
void left_truncate_QStringView_data() { left_data(); }
void left_truncate_QStringView() { left_impl<QStringView>(); }
void left_truncate_QLatin1String_data() { left_data(); }
void left_truncate_QLatin1String() { left_impl<QLatin1String>(); }
void left_truncate_QByteArray_data() { left_data(); }
void left_truncate_QByteArray() { left_impl<QByteArray>(); }
void left_QString_data() { left_data(); }
void left_QString() { left_impl<QString>(); }
void left_QStringRef_data() { left_data(); }
void left_QStringRef() { left_impl<QStringRef>(); }
void left_QStringView_data() { left_data(); }
void left_QStringView() { left_impl<QStringView>(); }
void left_QLatin1String_data() { left_data(); }
void left_QLatin1String() { left_impl<QLatin1String>(); }
void left_QByteArray_data();
void left_QByteArray() { left_impl<QByteArray>(); }
void right_QString_data() { right_data(); }
void right_QString() { right_impl<QString>(); }
@ -679,7 +676,7 @@ private Q_SLOTS:
void right_QStringView() { right_impl<QStringView>(); }
void right_QLatin1String_data() { right_data(); }
void right_QLatin1String() { right_impl<QLatin1String>(); }
void right_QByteArray_data() { right_data(); }
void right_QByteArray_data();
void right_QByteArray() { right_impl<QByteArray>(); }
void slice_QString_data() { slice_data(); }
@ -1533,6 +1530,37 @@ void tst_QStringApiSymmetry::tok_impl() const
void tst_QStringApiSymmetry::mid_data()
{
slice_data();
// mid() has a wider contract compared to slize(), so test those cases here:
#define ROW(base, p, n, r1, r2) \
QTest::addRow("%s %d %d", #base, p, n) << QStringRef(&base) << QLatin1String(#base) << p << n << QStringRef(&r1) << QStringRef(&r2)
ROW(a, -1, 0, a, null);
ROW(a, -1, 2, a, a);
ROW(a, -1, 3, a, a);
ROW(a, 0, -1, a, a);
ROW(a, 0, 2, a, a);
ROW(a, -1, -1, a, a);
ROW(a, 1, -1, empty, empty);
ROW(a, 1, 1, empty, empty);
ROW(a, 2, -1, null, null);
ROW(a, 2, 1, null, null);
ROW(abc, -1, -1, abc, abc);
ROW(abc, -1, 0, abc, null);
ROW(abc, -1, 2, abc, a);
ROW(abc, -1, 3, abc, ab);
ROW(abc, -1, 5, abc, abc);
ROW(abc, 0, -1, abc, abc);
ROW(abc, 0, 5, abc, abc);
ROW(abc, -1, 1, abc, null);
ROW(abc, -1, 2, abc, a);
ROW(abc, -1, 4, abc, abc);
ROW(abc, 1, -1, bc, bc);
ROW(abc, 1, 1, bc, b);
ROW(abc, 3, -1, empty, empty);
ROW(abc, 3, 1, empty, empty);
#undef ROW
}
template <typename String>
@ -1578,6 +1606,35 @@ void tst_QStringApiSymmetry::mid_impl()
void tst_QStringApiSymmetry::left_data()
{
first_data();
// specific data testing out of bounds cases
#define ROW(base, n, res) \
QTest::addRow("%s%d", #base, n) << QStringRef(&base) << QLatin1String(#base) << n << QStringRef(&res);
ROW(a, -1, a);
ROW(a, 2, a);
ROW(ab, -100, ab);
ROW(ab, 100, ab);
#undef ROW
}
// This is different from the rest for historical reasons. As we're replacing
// left() with first() as the recommended API, there's no point fixing this anymore
void tst_QStringApiSymmetry::left_QByteArray_data()
{
first_data();
// specific data testing out of bounds cases
#define ROW(base, n, res) \
QTest::addRow("%s%d", #base, n) << QStringRef(&base) << QLatin1String(#base) << n << QStringRef(&res);
ROW(a, -1, empty);
ROW(a, 2, a);
ROW(ab, -100, empty);
ROW(ab, 100, ab);
#undef ROW
}
template <typename String>
@ -1602,14 +1659,6 @@ void tst_QStringApiSymmetry::left_impl()
{
const auto left = detached(s).left(n);
QCOMPARE(left, result);
QCOMPARE(left.isNull(), result.isNull());
QCOMPARE(left.isEmpty(), result.isEmpty());
}
{
auto left = s;
left.truncate(n);
QCOMPARE(left, result);
QCOMPARE(left.isNull(), result.isNull());
QCOMPARE(left.isEmpty(), result.isEmpty());
@ -1619,6 +1668,35 @@ void tst_QStringApiSymmetry::left_impl()
void tst_QStringApiSymmetry::right_data()
{
last_data();
// specific data testing out of bounds cases
#define ROW(base, n, res) \
QTest::addRow("%s%d", #base, n) << QStringRef(&base) << QLatin1String(#base) << n << QStringRef(&res);
ROW(a, -1, a);
ROW(a, 2, a);
ROW(ab, -100, ab);
ROW(ab, 100, ab);
#undef ROW
}
// This is different from the rest for historical reasons. As we're replacing
// left() with first() as the recommended API, there's no point fixing this anymore
void tst_QStringApiSymmetry::right_QByteArray_data()
{
last_data();
// specific data testing out of bounds cases
#define ROW(base, n, res) \
QTest::addRow("%s%d", #base, n) << QStringRef(&base) << QLatin1String(#base) << n << QStringRef(&res);
ROW(a, -1, empty);
ROW(a, 2, a);
ROW(ab, -100, empty);
ROW(ab, 100, ab);
#undef ROW
}
template <typename String>
@ -1661,8 +1739,6 @@ void tst_QStringApiSymmetry::slice_data()
// QTest::addRow("null") << QStringRef() << QLatin1String() << 0 << 0 << QStringRef() << QStringRef();
QTest::addRow("empty") << QStringRef(&empty) << QLatin1String("") << 0 << 0 << QStringRef(&empty) << QStringRef(&empty);
// Some classes' mid() implementations have a wide contract, others a narrow one
// so only test valid arguents here:
#define ROW(base, p, n, r1, r2) \
QTest::addRow("%s%d%d", #base, p, n) << QStringRef(&base) << QLatin1String(#base) << p << n << QStringRef(&r1) << QStringRef(&r2)