QStringIterator: fix UB [2/2]: use std::less to compare pointers

The relational operators <, >, <=, >= do not form a total order for
pointers. In particular, they are only defined for pointer pairs that
point into the same array (incl. one past the array's last element).

Other uses are undefined behavior.

The iterators the class works on are user-supplied, so we cannot
assume that they are pointing into the same array. In fact, we have a
check in setPosition() that tries to catch such misuses, but similar
checks are absent from the ctors, e.g.

In addition, the implementation itself is checking for next-but-one
and prev-but-one in an unsafe way.

std::less can be used to provide a total order, so use that
everywhere.

Pick-to: 6.4 6.3 6.2 5.15
Change-Id: I32fb0ef9f768d9336ae12cc0542ca18d1bf23adf
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
bb10
Marc Mutz 2022-07-20 16:23:50 +02:00
parent 245da0762e
commit 7cf2684e6a
1 changed files with 8 additions and 5 deletions

View File

@ -25,6 +25,8 @@ class QStringIterator
{
QString::const_iterator i, pos, e;
static_assert((std::is_same<QString::const_iterator, const QChar *>::value));
static bool less(const QChar *lhs, const QChar *rhs) noexcept
{ return std::less{}(lhs, rhs); }
public:
explicit QStringIterator(QStringView string, qsizetype idx = 0)
: i(string.begin()),
@ -59,7 +61,8 @@ public:
inline void setPosition(QString::const_iterator position)
{
Q_ASSERT_X(i <= position && position <= e, Q_FUNC_INFO, "position out of bounds");
Q_ASSERT_X(!less(position, i) && !less(e, position),
Q_FUNC_INFO, "position out of bounds");
pos = position;
}
@ -67,7 +70,7 @@ public:
inline bool hasNext() const
{
return pos < e;
return less(pos, e);
}
inline void advance()
@ -95,7 +98,7 @@ public:
Q_ASSERT_X(hasNext(), Q_FUNC_INFO, "iterator hasn't a next item");
if (Q_UNLIKELY(pos->isHighSurrogate())) {
Q_ASSERT(pos + 1 < e && pos[1].isLowSurrogate());
Q_ASSERT(less(pos + 1, e) && pos[1].isLowSurrogate());
return QChar::surrogateToUcs4(pos[0], pos[1]);
}
@ -148,7 +151,7 @@ public:
inline bool hasPrevious() const
{
return pos > i;
return less(i, pos);
}
inline void recede()
@ -177,7 +180,7 @@ public:
Q_ASSERT_X(hasPrevious(), Q_FUNC_INFO, "iterator hasn't a previous item");
if (Q_UNLIKELY(pos[-1].isLowSurrogate())) {
Q_ASSERT(pos > i + 1 && pos[-2].isHighSurrogate());
Q_ASSERT(less(i + 1, pos) && pos[-2].isHighSurrogate());
return QChar::surrogateToUcs4(pos[-2], pos[-1]);
}
return pos[-1].unicode();