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

QLatin1String: perform the comparison to another QL1S using memcmp()

Browse Source 
qstrncmp() would stop at the first null character, which isn't correct.
The tests that had been disabled in tst_qstring.cpp (with an inaccurate
comment) were actually passing. I've added one more to ensure that the
terminating null is compared where needed.

[ChangeLog][QtCore][QLatin1String and QUtf8StringView] Fixed a
couple of bugs where two QLatin1Strings or two QUtf8StringViews
would stop their comparisons at the first embedded null
character, instead of comparing the full string. This issue
affected both classes' relational operators (less than, greater
than, etc.) and QUtf8StringView's operator== and operator!=.

Pick-to: 5.15 6.2 6.3
Change-Id: I0e5f6bec596a4a78bd3bfffd16c90ecea71ea68e
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
This commit is contained in:
Thiago Macieira 2022-01-10 16:10:19 -08:00 committed by Marc Mutz
parent 238e3beb6f
commit e52d50a03d
2 changed files with 37 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1444,7 +1444,7 @@ bool QtPrivate::equalStrings(QLatin1String lhs, QStringView rhs) noexcept
bool QtPrivate::equalStrings(QLatin1String lhs, QLatin1String rhs) noexcept
{
return lhs.size() == rhs.size() && (!lhs.size() || qstrncmp(lhs.data(), rhs.data(), lhs.size()) == 0);
return lhs.size() == rhs.size() && (!lhs.size() || memcmp(lhs.data(), rhs.data(), lhs.size()) == 0);
}
bool QtPrivate::equalStrings(QBasicUtf8StringView<false> lhs, QStringView rhs) noexcept
@ -1470,7 +1470,7 @@ bool QtPrivate::equalStrings(QBasicUtf8StringView<false> lhs, QLatin1String rhs)
bool QtPrivate::equalStrings(QBasicUtf8StringView<false> lhs, QBasicUtf8StringView<false> rhs) noexcept
{
return lhs.size() == rhs.size() && (!lhs.size() || qstrncmp(lhs.data(), rhs.data(), lhs.size()) == 0);
return lhs.size() == rhs.size() && (!lhs.size() || memcmp(lhs.data(), rhs.data(), lhs.size()) == 0);
}
bool QAnyStringView::equal(QAnyStringView lhs, QAnyStringView rhs) noexcept
@ -1577,7 +1577,7 @@ int QtPrivate::compareStrings(QLatin1String lhs, QLatin1String rhs, Qt::CaseSens
if (cs == Qt::CaseInsensitive)
return latin1nicmp(lhs.data(), lhs.size(), rhs.data(), rhs.size());
const auto l = std::min(lhs.size(), rhs.size());
int r = qstrncmp(lhs.data(), rhs.data(), l);
int r = memcmp(lhs.data(), rhs.data(), l);
return r ? r : lencmp(lhs.size(), rhs.size());
}
@ -1629,7 +1629,7 @@ int QtPrivate::compareStrings(QBasicUtf8StringView<false> lhs, QBasicUtf8StringV
if (cs == Qt::CaseInsensitive)
return compareStrings(lhs.toString(), rhs.toString(), cs); // ### optimize!
const auto l = std::min(lhs.size(), rhs.size());
int r = qstrncmp(lhs.data(), rhs.data(), l);
int r = memcmp(lhs.data(), rhs.data(), l);
return r ? r : lencmp(lhs.size(), rhs.size());
}

45
tests/auto/corelib/text/qstring/tst_qstring.cpp
View File

@ -6832,15 +6832,16 @@ void tst_QString::compare_data()
QTest::newRow("data8") << upper << lower << -1 << 0;
// embedded nulls
// These don't work as of now. It's OK that these don't work since \0 is not a valid unicode
/*QTest::newRow("data10") << QString(QByteArray("\0", 1)) << QString(QByteArray("\0", 1)) << 0 << 0;
QTest::newRow("data11") << QString(QByteArray("\0", 1)) << QString("") << 1 << 1;
QTest::newRow("data12") << QString("") << QString(QByteArray("\0", 1)) << -1 << -1;
QTest::newRow("data13") << QString("ab\0c") << QString(QByteArray("ab\0c", 4)) << 0 << 0;
QByteArray onenull("", 1);
QTest::newRow("data10") << QString(onenull) << QString(onenull) << 0 << 0;
QTest::newRow("data11") << QString(onenull) << QString("") << 1 << 1;
QTest::newRow("data12") << QString("") << QString(onenull) << -1 << -1;
QTest::newRow("data13") << QString::fromLatin1("ab\0c", 4) << QString(QByteArray("ab\0c", 4)) << 0 << 0;
QTest::newRow("data14") << QString(QByteArray("ab\0c", 4)) << QString("abc") << -1 << -1;
QTest::newRow("data15") << QString("abc") << QString(QByteArray("ab\0c", 4)) << 1 << 1;*/
QTest::newRow("data15") << QString("abc") << QString(QByteArray("ab\0c", 4)) << 1 << 1;
QTest::newRow("data16") << QString("abc") << QString(QByteArray("abc", 4)) << -1 << -1;
// All tests below (generated by the 3 for-loops) are meant to excercise the vectorized versions
// All tests below (generated by the 3 for-loops) are meant to exercise the vectorized versions
// of ucstrncmp.
QString in1, in2;
@ -6913,6 +6914,10 @@ void tst_QString::compare()
QByteArray s1_8 = s1.toUtf8();
QByteArray s2_8 = s2.toUtf8();
QByteArray s1_1 = s1.toLatin1();
QByteArray s2_1 = s2.toLatin1();
QLatin1String l1s1(s1_1);
QLatin1String l1s2(s2_1);
const QStringView v1(s1);
const QStringView v2(s2);
@ -6933,16 +6938,24 @@ void tst_QString::compare()
QCOMPARE(sign(QString::compare(s1, s2, Qt::CaseSensitive)), csr);
QCOMPARE(sign(QString::compare(s1, s2, Qt::CaseInsensitive)), cir);
if (csr == 0)
if (csr == 0) {
QVERIFY(qHash(s1) == qHash(s2));
QVERIFY(s1 == s2);
QVERIFY(!(s1 != s2));
} else {
QVERIFY(s1 != s2);
QVERIFY(!(s1 == s2));
}
if (!cir)
QCOMPARE(s1.toCaseFolded(), s2.toCaseFolded());
if (isLatin(s2)) {
QVERIFY(QtPrivate::isLatin1(s2));
QCOMPARE(sign(QString::compare(s1, QLatin1String(s2.toLatin1()))), csr);
QCOMPARE(sign(QString::compare(s1, QLatin1String(s2.toLatin1()), Qt::CaseInsensitive)), cir);
QCOMPARE(sign(QString::compare(s1, l1s2)), csr);
QCOMPARE(sign(QString::compare(s1, l1s2, Qt::CaseInsensitive)), cir);
// ensure it doesn't compare past the explicit size
QByteArray l1 = s2.toLatin1();
l1 += "x";
QLatin1String l1str(l1.constData(), l1.size() - 1);
@ -6952,8 +6965,16 @@ void tst_QString::compare()
if (isLatin(s1)) {
QVERIFY(QtPrivate::isLatin1(s1));
QCOMPARE(sign(QString::compare(QLatin1String(s1.toLatin1()), s2)), csr);
QCOMPARE(sign(QString::compare(QLatin1String(s1.toLatin1()), s2, Qt::CaseInsensitive)), cir);
QCOMPARE(sign(QString::compare(l1s1, s2)), csr);
QCOMPARE(sign(QString::compare(l1s1, s2, Qt::CaseInsensitive)), cir);
}
if (isLatin(s1) && isLatin(s2)) {
QCOMPARE(sign(QtPrivate::compareStrings(l1s1, l1s2)), csr);
QCOMPARE(sign(QtPrivate::compareStrings(l1s1, l1s2, Qt::CaseInsensitive)), cir);
QCOMPARE(l1s1 == l1s2, csr == 0);
QCOMPARE(l1s1 < l1s2, csr < 0);
QCOMPARE(l1s1 > l1s2, csr > 0);
}
}