From 49113c905d5868e6b76bb6b7b3e0a20b0c56a23a Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Mon, 1 Mar 2021 11:44:28 +0100 Subject: Don't access moved-from object Clang's static analyzer tooling warns about this access-after-move [1]. While the comment above the function indicated that this was deliberate and relying on a moved-from QString being valid, it is still bad practice. Since 'str' is empty in moved-from state if - and only if - it was a non-const reference, make a compile-time check of the constness of type T instead. [1] https://testresults.qt.io/codechecker/daily_analyses/qtbase/dev/qtbase-dev-20210301-e14ccf0553/qstring.cpp_clang-tidy_b0545db57a2cc5dac67a56f76322ffd0.plist.html#reportHash=209ee3db0b17d21919326a1ad6635318 Pick-to: 6.1 Change-Id: Iac1813b61b6a3c2ef4053b911a4043c5382f85e4 Reviewed-by: Thiago Macieira Reviewed-by: Edward Welbourne Reviewed-by: Andrei Golubev --- src/corelib/text/qstring.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'src/corelib') diff --git a/src/corelib/text/qstring.cpp b/src/corelib/text/qstring.cpp index a6f3cd2b5d..390c10e1fc 100644 --- a/src/corelib/text/qstring.cpp +++ b/src/corelib/text/qstring.cpp @@ -6342,9 +6342,9 @@ namespace QUnicodeTables { \endlist In copy-convert mode, the local variable \c{s} is detached from the input - \a str. In the in-place convert mode, \a str is in moved-from state (which - this function requires to be a valid, empty string) and \c{s} contains the - only copy of the string, without reallocation (thus, \a it is still valid). + \a str. In the in-place convert mode, \a str is in moved-from state and + \c{s} contains the only copy of the string, without reallocation (thus, + \a it is still valid). There is one pathological case left: when the in-place conversion needs to reallocate memory to grow the buffer. In that case, we need to adjust the \a @@ -6355,7 +6355,7 @@ Q_NEVER_INLINE static QString detachAndConvertCase(T &str, QStringIterator it, QUnicodeTables::Case which) { Q_ASSERT(!str.isEmpty()); - QString s = std::move(str); // will copy if T is const QString + QString s = std::move(str); // will copy if T is const QString QChar *pp = s.begin() + it.index(); // will detach if necessary do { @@ -6374,9 +6374,8 @@ static QString detachAndConvertCase(T &str, QStringIterator it, QUnicodeTables:: s.replace(outpos, 1, reinterpret_cast(folded.data()), folded.size()); pp = const_cast(s.constBegin()) + outpos + folded.size(); - // do we need to adjust the input iterator too? - // if it is pointing to s's data, str is empty - if (str.isEmpty()) + // Adjust the input iterator if we are performing an in-place conversion + if constexpr (!std::is_const::value) it = QStringIterator(s.constBegin(), inpos + folded.size(), s.constEnd()); } } else { -- cgit v1.2.3