summaryrefslogtreecommitdiffstats
path: root/src/corelib/text/qstring.cpp
diff options
context:
space:
mode:
authorGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2022-07-06 02:20:38 +0200
committerGiuseppe D'Angelo <giuseppe.dangelo@kdab.com>2022-07-10 23:40:24 +0200
commit6f852935e0c847f91c7d6ffca455a74307ebe26d (patch)
tree037cd5c792abf48ef86535f7a249bffef84e5913 /src/corelib/text/qstring.cpp
parent54b276be0b5885bbaee2c38f472eb39731fd684a (diff)
QString: fix lifetime issues with QRegularExpression APIs
QString has several functions taking a QRegularExpression: indexOf(), contains(), and so on. Some of those have an out-argument of type QRegularExpressionMatch, to report the details of the match (if any). For instance: QRegularExpression re(...); QRegularExpressionMatch match; if (string.contains(re, &match)) use(match); The code used to route the implementation of these functions through QStringView (which has the very same functions). This however opens up a lifetime problem with temporary strings: if (getString().contains(re, &match)) use(match); // match is dangling Here `match` is dangling because it is referencing data into the destroyed temporary -- nothing is keeping the string alive. This is against the rules we've decided for Qt, and it's also asymmetric with the corresponding code that uses QRegularExpression directly instead: match = re.match(getString()); if (match.hasMatch()) use(match); // not dangling ... although we've documented not to do this. (In light of the decision we've made w.r.t. temporaries, the documentation is wrong anyways.) Here QRE takes a copy of the string and stores it in the match object, thus keeping it alive. Hence, extend the implementation of the QString functions to keep a (shallow) copy of the string. To keep the code shared as much as possible with QStringView, in theory one could have a function taking a std::variant<QString, QStringView> and that uses the currently active member. However I've found that std::variant here creates some abysmal codegen, so instead I went for a simpler approach -- pass a QStringView and an optional pointer to a QString. Use the latter if it's loaded. QStringView has some inline code that calls into exported functions, so I can't change the signature of them without breaking BC; I'm instead adding new unexported functions and a Qt 7 note to unify them. Change-Id: I7c65885a84069d0fbb902dcc96ddff543ca84562 Fixes: QTBUG-103940 Pick-to: 6.2 6.3 6.4 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Diffstat (limited to 'src/corelib/text/qstring.cpp')
-rw-r--r--src/corelib/text/qstring.cpp41
1 files changed, 31 insertions, 10 deletions
diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp
index 1694ea8dbe..c569bf627d 100644
--- a/src/corelib/text/qstring.cpp
+++ b/src/corelib/text/qstring.cpp
@@ -4560,7 +4560,7 @@ qsizetype QString::count(QStringView str, Qt::CaseSensitivity cs) const
*/
qsizetype QString::indexOf(const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch) const
{
- return QtPrivate::indexOf(QStringView(*this), re, from, rmatch);
+ return QtPrivate::indexOf(QStringView(*this), this, re, from, rmatch);
}
/*!
@@ -4594,7 +4594,7 @@ qsizetype QString::indexOf(const QRegularExpression &re, qsizetype from, QRegula
*/
qsizetype QString::lastIndexOf(const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch) const
{
- return QtPrivate::lastIndexOf(QStringView(*this), re, from, rmatch);
+ return QtPrivate::lastIndexOf(QStringView(*this), this, re, from, rmatch);
}
/*!
@@ -4633,7 +4633,7 @@ qsizetype QString::lastIndexOf(const QRegularExpression &re, qsizetype from, QRe
bool QString::contains(const QRegularExpression &re, QRegularExpressionMatch *rmatch) const
{
- return QtPrivate::contains(QStringView(*this), re, rmatch);
+ return QtPrivate::contains(QStringView(*this), this, re, rmatch);
}
/*!
@@ -10798,14 +10798,16 @@ qsizetype QtPrivate::lastIndexOf(QLatin1StringView haystack, qsizetype from, QLa
}
#if QT_CONFIG(regularexpression)
-qsizetype QtPrivate::indexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+qsizetype QtPrivate::indexOf(QStringView viewHaystack, const QString *stringHaystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
{
if (!re.isValid()) {
qtWarnAboutInvalidRegularExpression(re.pattern(), "QString(View)::indexOf");
return -1;
}
- QRegularExpressionMatch match = re.match(haystack, from);
+ QRegularExpressionMatch match = stringHaystack
+ ? re.match(*stringHaystack, from)
+ : re.match(viewHaystack, from);
if (match.hasMatch()) {
const qsizetype ret = match.capturedStart();
if (rmatch)
@@ -10816,15 +10818,22 @@ qsizetype QtPrivate::indexOf(QStringView haystack, const QRegularExpression &re,
return -1;
}
-qsizetype QtPrivate::lastIndexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+qsizetype QtPrivate::indexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+{
+ return indexOf(haystack, nullptr, re, from, rmatch);
+}
+
+qsizetype QtPrivate::lastIndexOf(QStringView viewHaystack, const QString *stringHaystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
{
if (!re.isValid()) {
qtWarnAboutInvalidRegularExpression(re.pattern(), "QString(View)::lastIndexOf");
return -1;
}
- qsizetype endpos = (from < 0) ? (haystack.size() + from + 1) : (from + 1);
- QRegularExpressionMatchIterator iterator = re.globalMatch(haystack);
+ qsizetype endpos = (from < 0) ? (viewHaystack.size() + from + 1) : (from + 1);
+ QRegularExpressionMatchIterator iterator = stringHaystack
+ ? re.globalMatch(*stringHaystack)
+ : re.globalMatch(viewHaystack);
qsizetype lastIndex = -1;
while (iterator.hasNext()) {
QRegularExpressionMatch match = iterator.next();
@@ -10841,19 +10850,31 @@ qsizetype QtPrivate::lastIndexOf(QStringView haystack, const QRegularExpression
return lastIndex;
}
-bool QtPrivate::contains(QStringView haystack, const QRegularExpression &re, QRegularExpressionMatch *rmatch)
+qsizetype QtPrivate::lastIndexOf(QStringView haystack, const QRegularExpression &re, qsizetype from, QRegularExpressionMatch *rmatch)
+{
+ return lastIndexOf(haystack, nullptr, re, from, rmatch);
+}
+
+bool QtPrivate::contains(QStringView viewHaystack, const QString *stringHaystack, const QRegularExpression &re, QRegularExpressionMatch *rmatch)
{
if (!re.isValid()) {
qtWarnAboutInvalidRegularExpression(re.pattern(), "QString(View)::contains");
return false;
}
- QRegularExpressionMatch m = re.match(haystack);
+ QRegularExpressionMatch m = stringHaystack
+ ? re.match(*stringHaystack)
+ : re.match(viewHaystack);
bool hasMatch = m.hasMatch();
if (hasMatch && rmatch)
*rmatch = std::move(m);
return hasMatch;
}
+bool QtPrivate::contains(QStringView haystack, const QRegularExpression &re, QRegularExpressionMatch *rmatch)
+{
+ return contains(haystack, nullptr, re, rmatch);
+}
+
qsizetype QtPrivate::count(QStringView haystack, const QRegularExpression &re)
{
if (!re.isValid()) {