From 6bafb9da71044d99a95a591f7872b8b2fd97174a Mon Sep 17 00:00:00 2001 From: Anton Kudryavtsev Date: Thu, 7 Jul 2016 11:05:07 +0300 Subject: QString: fix incomplete doc of chop() Change-Id: I84de848681e793e68e0c290719a7f961aca48f4e Reviewed-by: Edward Welbourne Reviewed-by: Thiago Macieira --- src/corelib/tools/qstring.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/tools/qstring.cpp') diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index 8eadc9330d..3d63a540dd 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -4978,7 +4978,7 @@ void QString::truncate(int pos) Removes \a n characters from the end of the string. If \a n is greater than or equal to size(), the result is an - empty string. + empty string; if \a n is negative, it is equivalent to passing zero. Example: \snippet qstring/main.cpp 15 -- cgit v1.2.3 From 9a3bcd1c3d4914a9594d80dc0c3812fd88be42c3 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Wed, 30 Mar 2016 19:26:29 +0200 Subject: QString::replace(): Commentary clarifications. Also skip doing a += 0 when we had to test whether the relevant rhs was zero anyway (because we want to ++ there instead of +=ing). Change-Id: Ibd5f21eb9aaf410b09c9db8450b2d61618e628fc Reviewed-by: Robin Burchell --- src/corelib/tools/qstring.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/corelib/tools/qstring.cpp') diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index 3d63a540dd..68681a90d0 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -2488,19 +2488,19 @@ QString &QString::replace(const QChar *before, int blen, if (index == -1) break; indices[pos++] = index; - index += blen; - // avoid infinite loop - if (!blen) + if (blen) // Step over before: + index += blen; + else // Only count one instance of empty between any two characters: index++; } - if (!pos) + if (!pos) // Nothing to replace break; replace_helper(indices, pos, blen, after, alen); - if (index == -1) + if (index == -1) // Nothing left to replace break; - // index has to be adjusted in case we get back into the loop above. + // The call to replace_helper just moved what index points at: index += pos*(alen-blen); } @@ -2545,14 +2545,14 @@ QString& QString::replace(QChar ch, const QString &after, Qt::CaseSensitivity cs index++; } } - if (!pos) + if (!pos) // Nothing to replace break; replace_helper(indices, pos, 1, after.constData(), after.d->size); - if (index == -1) + if (index == -1) // Nothing left to replace break; - // index has to be adjusted in case we get back into the loop above. + // The call to replace_helper just moved what index points at: index += pos*(after.d->size - 1); } return *this; -- cgit v1.2.3 From e21bf5e6b390001ac83c403005957ddc8bfc36ae Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Wed, 30 Mar 2016 16:01:27 +0200 Subject: QString::replace(): protect sought text and replacement When replacing each copy of one text with a copy of another, we do so in batches of 1024; if we get more than one batch, we need to keep a copy of the sought text and replacement if they're part of the string we're modifying, for use in later batches. Also do the replacements in full batches of 1024, not 1023 (which left the last entry in an array unused); marked some related tests as (un)likely; and move some repeated code out into a pair of little local functions to save duplcation. Those new functions can also serve replace_helper(); and it can shed a const_cast and some conditioning of free() by using them the same way replace() now does. (There was also one place it still used the raw after, rather than the replacement copy; which could have produced errors if memcpy were to exercise its right to assume no overlap in arrays. This error is what prompted me to notice all of the above.) Added tests. The last error proved untestable as my memcpy is in fact as fussy as memmove. The first two tests added were attempts to get a failure out of it. The third did get a failure, but also tripped over the problem in replace() itself. Added to an existing test function and renamed it to generally cover extra tests for replace. Change-Id: I9ba6928c84ece266dbbe52b91e333ea54ab6d95e Reviewed-by: Robin Burchell Reviewed-by: Lars Knoll --- src/corelib/tools/qstring.cpp | 69 ++++++++++++++++++++++++++++++------------- 1 file changed, 49 insertions(+), 20 deletions(-) (limited to 'src/corelib/tools/qstring.cpp') diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index 68681a90d0..b5119444b7 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -2381,26 +2381,40 @@ QString &QString::replace(const QString &before, const QString &after, Qt::CaseS return replace(before.constData(), before.size(), after.constData(), after.size(), cs); } +namespace { // helpers for replace and its helper: +QChar *textCopy(const QChar *start, int len) +{ + const size_t size = len * sizeof(QChar); + QChar *const copy = static_cast(::malloc(size)); + Q_CHECK_PTR(copy); + ::memcpy(copy, start, size); + return copy; +} + +bool pointsIntoRange(const QChar *ptr, const ushort *base, int len) +{ + const QChar *const start = reinterpret_cast(base); + return start <= ptr && ptr < start + len; +} +} // end namespace + /*! \internal */ void QString::replace_helper(uint *indices, int nIndices, int blen, const QChar *after, int alen) { - // copy *after in case it lies inside our own d->data() area - // (which we could possibly invalidate via a realloc or corrupt via memcpy operations.) - QChar *afterBuffer = const_cast(after); - if (after >= reinterpret_cast(d->data()) && after < reinterpret_cast(d->data()) + d->size) { - afterBuffer = static_cast(::malloc(alen*sizeof(QChar))); - Q_CHECK_PTR(afterBuffer); - ::memcpy(afterBuffer, after, alen*sizeof(QChar)); - } + // Copy after if it lies inside our own d->data() area (which we could + // possibly invalidate via a realloc or modify by replacement). + QChar *afterBuffer = 0; + if (pointsIntoRange(after, d->data(), d->size)) // Use copy in place of vulnerable original: + after = afterBuffer = textCopy(after, alen); QT_TRY { if (blen == alen) { // replace in place detach(); for (int i = 0; i < nIndices; ++i) - memcpy(d->data() + indices[i], afterBuffer, alen * sizeof(QChar)); + memcpy(d->data() + indices[i], after, alen * sizeof(QChar)); } else if (alen < blen) { // replace from front detach(); @@ -2416,7 +2430,7 @@ void QString::replace_helper(uint *indices, int nIndices, int blen, const QChar to += msize; } if (alen) { - memcpy(d->data() + to, afterBuffer, alen*sizeof(QChar)); + memcpy(d->data() + to, after, alen * sizeof(QChar)); to += alen; } movestart = indices[i] + blen; @@ -2439,17 +2453,15 @@ void QString::replace_helper(uint *indices, int nIndices, int blen, const QChar int moveto = insertstart + alen; memmove(d->data() + moveto, d->data() + movestart, (moveend - movestart)*sizeof(QChar)); - memcpy(d->data() + insertstart, afterBuffer, alen*sizeof(QChar)); + memcpy(d->data() + insertstart, after, alen * sizeof(QChar)); moveend = movestart-blen; } } } QT_CATCH(const std::bad_alloc &) { - if (afterBuffer != after) - ::free(afterBuffer); + ::free(afterBuffer); QT_RETHROW; } - if (afterBuffer != after) - ::free(afterBuffer); + ::free(afterBuffer); } /*! @@ -2478,12 +2490,13 @@ QString &QString::replace(const QChar *before, int blen, return *this; QStringMatcher matcher(before, blen, cs); + QChar *beforeBuffer = 0, *afterBuffer = 0; int index = 0; while (1) { uint indices[1024]; uint pos = 0; - while (pos < 1023) { + while (pos < 1024) { index = matcher.indexIn(*this, index); if (index == -1) break; @@ -2496,13 +2509,29 @@ QString &QString::replace(const QChar *before, int blen, if (!pos) // Nothing to replace break; + if (Q_UNLIKELY(index != -1)) { + /* + We're about to change data, that before and after might point + into, and we'll need that data for our next batch of indices. + */ + if (!afterBuffer && pointsIntoRange(after, d->data(), d->size)) + after = afterBuffer = textCopy(after, alen); + + if (!beforeBuffer && pointsIntoRange(before, d->data(), d->size)) { + beforeBuffer = textCopy(before, blen); + matcher = QStringMatcher(beforeBuffer, blen, cs); + } + } + replace_helper(indices, pos, blen, after, alen); - if (index == -1) // Nothing left to replace + if (Q_LIKELY(index == -1)) // Nothing left to replace break; // The call to replace_helper just moved what index points at: index += pos*(alen-blen); } + ::free(afterBuffer); + ::free(beforeBuffer); return *this; } @@ -2533,13 +2562,13 @@ QString& QString::replace(QChar ch, const QString &after, Qt::CaseSensitivity cs uint indices[1024]; uint pos = 0; if (cs == Qt::CaseSensitive) { - while (pos < 1023 && index < d->size) { + while (pos < 1024 && index < d->size) { if (d->data()[index] == cc) indices[pos++] = index; index++; } } else { - while (pos < 1023 && index < d->size) { + while (pos < 1024 && index < d->size) { if (QChar::toCaseFolded(d->data()[index]) == cc) indices[pos++] = index; index++; @@ -2550,7 +2579,7 @@ QString& QString::replace(QChar ch, const QString &after, Qt::CaseSensitivity cs replace_helper(indices, pos, 1, after.constData(), after.d->size); - if (index == -1) // Nothing left to replace + if (Q_LIKELY(index == -1)) // Nothing left to replace break; // The call to replace_helper just moved what index points at: index += pos*(after.d->size - 1); -- cgit v1.2.3 From 5c46d07a7ce1542301b6e428f9f8d07738e7518f Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 12 Jul 2016 12:37:16 +0200 Subject: Optimize QString::insert() When the insertion position is not beyond end(), call resize() instead of expand(), which fills the new size with spaces, which, however would just be overwritten by the following memmove(). Add some Q_UNLIKELY to indicate that we strongly expect the resize() case to be the more common. Change-Id: Iaf3215dd53c2cbd18f2fd8a5f80af8f6844944da Reviewed-by: Edward Welbourne Reviewed-by: Anton Kudryavtsev Reviewed-by: Thiago Macieira --- src/corelib/tools/qstring.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'src/corelib/tools/qstring.cpp') diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index 629a0bc744..9dc7136d2a 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -1985,7 +1985,10 @@ QString &QString::insert(int i, QLatin1String str) return *this; int len = str.size(); - expand(qMax(d->size, i) + len - 1); + if (Q_UNLIKELY(i > d->size)) + expand(i + len - 1); + else + resize(d->size + len); ::memmove(d->data() + i + len, d->data() + i, (d->size - i - len) * sizeof(QChar)); qt_from_latin1(d->data() + i, s, uint(len)); @@ -2015,7 +2018,10 @@ QString& QString::insert(int i, const QChar *unicode, int size) return *this; } - expand(qMax(d->size, i) + size - 1); + if (Q_UNLIKELY(i > d->size)) + expand(i + size - 1); + else + resize(d->size + size); ::memmove(d->data() + i + size, d->data() + i, (d->size - i - size) * sizeof(QChar)); memcpy(d->data() + i, s, size * sizeof(QChar)); @@ -2035,7 +2041,10 @@ QString& QString::insert(int i, QChar ch) i += d->size; if (i < 0) return *this; - expand(qMax(i, d->size)); + if (Q_UNLIKELY(i > d->size)) + expand(i); + else + resize(d->size + 1); ::memmove(d->data() + i + 1, d->data() + i, (d->size - i - 1) * sizeof(QChar)); d->data()[i] = ch.unicode(); return *this; -- cgit v1.2.3 From 5d935dca0c70618af519c5bef2e11b4a1ee03507 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 12 Jul 2016 12:44:01 +0200 Subject: Remove (private) QString::expand(), port to (new) QString::resize(int, QChar) We cannot really remove the function, since it's called from inline code (QCharRef::op=(QChar)), but we can schedule it for removal in Qt 6, and inline it into existing in-tree callers. Change-Id: I3499f101dcb5ae908726b3673bf3526a04408db6 Reviewed-by: Edward Welbourne Reviewed-by: Thiago Macieira --- src/corelib/tools/qstring.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'src/corelib/tools/qstring.cpp') diff --git a/src/corelib/tools/qstring.cpp b/src/corelib/tools/qstring.cpp index 9dc7136d2a..68d12d85af 100644 --- a/src/corelib/tools/qstring.cpp +++ b/src/corelib/tools/qstring.cpp @@ -1789,17 +1789,12 @@ void QString::reallocData(uint alloc, bool grow) } } +#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0) void QString::expand(int i) { - int sz = d->size; - resize(qMax(i + 1, sz)); - if (d->size - 1 > sz) { - ushort *n = d->data() + d->size - 1; - ushort *e = d->data() + sz; - while (n != e) - * --n = ' '; - } + resize(qMax(i + 1, d->size), QLatin1Char(' ')); } +#endif /*! \fn void QString::clear() @@ -1986,7 +1981,7 @@ QString &QString::insert(int i, QLatin1String str) int len = str.size(); if (Q_UNLIKELY(i > d->size)) - expand(i + len - 1); + resize(i + len, QLatin1Char(' ')); else resize(d->size + len); @@ -2019,7 +2014,7 @@ QString& QString::insert(int i, const QChar *unicode, int size) } if (Q_UNLIKELY(i > d->size)) - expand(i + size - 1); + resize(i + size, QLatin1Char(' ')); else resize(d->size + size); @@ -2042,7 +2037,7 @@ QString& QString::insert(int i, QChar ch) if (i < 0) return *this; if (Q_UNLIKELY(i > d->size)) - expand(i); + resize(i + 1, QLatin1Char(' ')); else resize(d->size + 1); ::memmove(d->data() + i + 1, d->data() + i, (d->size - i - 1) * sizeof(QChar)); -- cgit v1.2.3