Q(Static)ByteArrayMatcher: manage indexIn() overloads

Unlike QString and QStringView, QByteArrayView and QByteArray don't
overload well.

Solve the overload issue the usual way: by making the QByteArray one a
Q_WEAK_OVERLOAD. This is trivial for QStaticByteArrayMatcher, which
isn't exported, but require QT_REMOVED_SINCE magic for
QByteArrayMatcher, which is.

The additional const char* overload has shielded us from the worst
fall-out so far, it seems, but it makes for a truly horrible overload
set:

    matcher.indexIn(str, 3);

Q: Is the 3 here the length of the haystack or the value of the from
parameter?

A: It depends on decltype(str)!

If the (const char*, qsizetype, qsizetype=0) overload is the better
match, then 3 limits the haystack's length.

If, otoh, the (QByteArray(View), qsizetype) overload is the better
match, then it's the value of the from parameter.

As if this wasn't bad enough, QByteArray implcitly converts to const
char* by default!

A follow-up patch will therefore deprecate the (ptr, size) overloads,
so we de-inline the QByteArrayView ones to avoid having to touch the
implementation once more.

Found during 6.3 API review.

Pick-to: 6.3
Change-Id: I9640e0bdd298d651511adebcc85f314db9221d34
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2022-01-21 18:06:24 +01:00
parent 3d3558dc8f
commit cce7e35253
4 changed files with 56 additions and 19 deletions

View File

@ -75,6 +75,11 @@ int QStaticByteArrayMatcherBase::indexOfIn(const char *h, uint hl, const char *n
# endif // QT_POINTER_SIZE != 4 # endif // QT_POINTER_SIZE != 4
qsizetype QByteArrayMatcher::indexIn(const QByteArray &ba, qsizetype from) const
{
return indexIn(QByteArrayView{ba}, from); // ba.isNull() may be significant, so don't ignore it!
}
#include "tools/qcryptographichash.h" #include "tools/qcryptographichash.h"
void QCryptographicHash::addData(const QByteArray &data) void QCryptographicHash::addData(const QByteArray &data)

View File

@ -205,21 +205,6 @@ void QByteArrayMatcher::setPattern(const QByteArray &pattern)
bm_init_skiptable(p.p, p.l, p.q_skiptable); bm_init_skiptable(p.p, p.l, p.q_skiptable);
} }
/*!
Searches the byte array \a ba, from byte position \a from (default
0, i.e. from the first byte), for the byte array pattern() that
was set in the constructor or in the most recent call to
setPattern(). Returns the position where the pattern() matched in
\a ba, or -1 if no match was found.
*/
qsizetype QByteArrayMatcher::indexIn(const QByteArray &ba, qsizetype from) const
{
if (from < 0)
from = 0;
return bm_find(reinterpret_cast<const uchar *>(ba.constData()), ba.size(), from,
p.p, p.l, p.q_skiptable);
}
/*! /*!
Searches the char string \a str, which has length \a len, from Searches the char string \a str, which has length \a len, from
byte position \a from (default 0, i.e. from the first byte), for byte position \a from (default 0, i.e. from the first byte), for
@ -246,6 +231,13 @@ qsizetype QByteArrayMatcher::indexIn(const char *str, qsizetype len, qsizetype f
setPattern(). Returns the position where the pattern() matched in setPattern(). Returns the position where the pattern() matched in
\a data, or -1 if no match was found. \a data, or -1 if no match was found.
*/ */
qsizetype QByteArrayMatcher::indexIn(QByteArrayView data, qsizetype from) const
{
if (from < 0)
from = 0;
return bm_find(reinterpret_cast<const uchar *>(data.data()), data.size(), from,
p.p, p.l, p.q_skiptable);
}
/*! /*!
\fn QByteArray QByteArrayMatcher::pattern() const \fn QByteArray QByteArrayMatcher::pattern() const

View File

@ -65,12 +65,15 @@ public:
void setPattern(const QByteArray &pattern); void setPattern(const QByteArray &pattern);
#if QT_REMOVED_SINCE(6, 3)
qsizetype indexIn(const QByteArray &ba, qsizetype from = 0) const; qsizetype indexIn(const QByteArray &ba, qsizetype from = 0) const;
#else
Q_WEAK_OVERLOAD
qsizetype indexIn(const QByteArray &ba, qsizetype from = 0) const
{ return indexIn(QByteArrayView{ba}, from); }
#endif
qsizetype indexIn(const char *str, qsizetype len, qsizetype from = 0) const; qsizetype indexIn(const char *str, qsizetype len, qsizetype from = 0) const;
qsizetype indexIn(QByteArrayView data, qsizetype from = 0) const qsizetype indexIn(QByteArrayView data, qsizetype from = 0) const;
{
return indexIn(data.data(), data.size(), from);
}
inline QByteArray pattern() const inline QByteArray pattern() const
{ {
if (q_pattern.isNull()) if (q_pattern.isNull())
@ -162,10 +165,13 @@ public:
m_pattern[i] = patternToMatch[i]; m_pattern[i] = patternToMatch[i];
} }
Q_WEAK_OVERLOAD
qsizetype indexIn(const QByteArray &haystack, qsizetype from = 0) const noexcept qsizetype indexIn(const QByteArray &haystack, qsizetype from = 0) const noexcept
{ return this->indexOfIn(m_pattern, N - 1, haystack.data(), haystack.size(), from); } { return this->indexOfIn(m_pattern, N - 1, haystack.data(), haystack.size(), from); }
qsizetype indexIn(const char *haystack, qsizetype hlen, qsizetype from = 0) const noexcept qsizetype indexIn(const char *haystack, qsizetype hlen, qsizetype from = 0) const noexcept
{ return this->indexOfIn(m_pattern, N - 1, haystack, hlen, from); } { return this->indexOfIn(m_pattern, N - 1, haystack, hlen, from); }
qsizetype indexIn(QByteArrayView haystack, qsizetype from = 0) const noexcept
{ return this->indexOfIn(m_pattern, N - 1, haystack.data(), haystack.size(), from); }
QByteArray pattern() const { return QByteArray(m_pattern, qsizetype(N - 1)); } QByteArray pattern() const { return QByteArray(m_pattern, qsizetype(N - 1)); }
}; };

View File

@ -48,12 +48,46 @@ class tst_QByteArrayMatcher : public QObject
Q_OBJECT Q_OBJECT
private slots: private slots:
void overloads();
void interface(); void interface();
void indexIn(); void indexIn();
void staticByteArrayMatcher(); void staticByteArrayMatcher();
void haystacksWithMoreThan4GiBWork(); void haystacksWithMoreThan4GiBWork();
}; };
void tst_QByteArrayMatcher::overloads()
{
QByteArray hello = QByteArrayLiteral("hello");
QByteArray hello2 = hello.repeated(2);
{
QByteArrayMatcher m("hello");
QCOMPARE(m.pattern(), "hello");
QCOMPARE(m.indexIn("hello"), 0);
}
{
QByteArrayMatcher m("hello", qsizetype(3));
QCOMPARE(m.pattern(), "hel");
QCOMPARE(m.indexIn("hellohello", qsizetype(2)), -1); // haystack is "he", not: from is 2
QCOMPARE(m.indexIn("hellohello", qsizetype(3)), 0); // haystack is "hel", not: from is 3
}
{
QByteArrayMatcher m(hello);
QCOMPARE(m.pattern(), "hello");
QCOMPARE(m.indexIn(hello), 0);
QCOMPARE(m.indexIn(hello2, qsizetype(1)), hello.size());
}
{
QStaticByteArrayMatcher m("hel");
QCOMPARE(m.pattern(), "hel");
QCOMPARE(m.indexIn("hello"), qsizetype(0));
QCOMPARE(m.indexIn("hellohello", qsizetype(2)), -1); // haystack is "he", not: from is 2
QCOMPARE(m.indexIn("hellohello", qsizetype(3)), 0); // haystack is "hel", not: from is 3
QCOMPARE(m.indexIn(hello), 0);
QCOMPARE(m.indexIn(hello2, qsizetype(2)), hello.size()); // from is 2
QCOMPARE(m.indexIn(hello2, qsizetype(3)), hello.size()); // from is 3
}
}
void tst_QByteArrayMatcher::interface() void tst_QByteArrayMatcher::interface()
{ {
const char needle[] = "abc123"; const char needle[] = "abc123";