From cb5a50c6a7c91b5a09b3bef63f9529a6f77f0114 Mon Sep 17 00:00:00 2001 From: chrchr-github <78114321+chrchr-github@users.noreply.github.com> Date: Tue, 18 Jan 2022 20:17:05 +0100 Subject: [PATCH] Fix #10710 FN passedByValue with QString (#3696) --- gui/mainwindow.cpp | 4 ++-- gui/mainwindow.h | 4 ++-- gui/projectfile.cpp | 4 ++-- gui/projectfile.h | 10 +++++----- gui/variablecontractsdialog.cpp | 2 +- lib/checkother.cpp | 21 +++++++++++++++------ test/cfg/qt.cpp | 16 ++++++++++++++++ test/cfg/std.cpp | 2 ++ 8 files changed, 45 insertions(+), 18 deletions(-) diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index da37e211b..ca0b14cae 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -1918,7 +1918,7 @@ void MainWindow::editVariableContract(QString var) updateVariableContractsTab(); } -void MainWindow::deleteFunctionContract(QString function) +void MainWindow::deleteFunctionContract(const QString& function) { if (mProjectFile) { mProjectFile->deleteFunctionContract(function); @@ -1926,7 +1926,7 @@ void MainWindow::deleteFunctionContract(QString function) } } -void MainWindow::deleteVariableContract(QString var) +void MainWindow::deleteVariableContract(const QString& var) { if (mProjectFile) { mProjectFile->deleteVariableContract(var); diff --git a/gui/mainwindow.h b/gui/mainwindow.h index d10836c0d..3348cfa43 100644 --- a/gui/mainwindow.h +++ b/gui/mainwindow.h @@ -234,10 +234,10 @@ protected slots: void editVariableContract(QString var); /** Delete contract for function */ - void deleteFunctionContract(QString function); + void deleteFunctionContract(const QString& function); /** Edit constraints for variable */ - void deleteVariableContract(QString var); + void deleteVariableContract(const QString& var); private: diff --git a/gui/projectfile.cpp b/gui/projectfile.cpp index 983500bda..9471ffac7 100644 --- a/gui/projectfile.cpp +++ b/gui/projectfile.cpp @@ -788,7 +788,7 @@ void ProjectFile::setLibraries(const QStringList &libraries) mLibraries = libraries; } -void ProjectFile::setFunctionContract(QString function, QString expects) +void ProjectFile::setFunctionContract(const QString& function, const QString& expects) { mFunctionContracts[function.toStdString()] = expects.toStdString(); } @@ -818,7 +818,7 @@ void ProjectFile::setVSConfigurations(const QStringList &vsConfigs) mVsConfigurations = vsConfigs; } -void ProjectFile::setWarningTags(std::size_t hash, QString tags) +void ProjectFile::setWarningTags(std::size_t hash, const QString& tags) { if (tags.isEmpty()) mWarningTags.erase(hash); diff --git a/gui/projectfile.h b/gui/projectfile.h index 94f74ac2e..2341634bc 100644 --- a/gui/projectfile.h +++ b/gui/projectfile.h @@ -236,15 +236,15 @@ public: return mVariableContracts; } - void setVariableContracts(QString var, QString min, QString max) { + void setVariableContracts(QString var, const QString& min, const QString& max) { mVariableContracts[var] = Settings::VariableContracts{min.toStdString(), max.toStdString()}; } - void deleteFunctionContract(QString function) { + void deleteFunctionContract(const QString& function) { mFunctionContracts.erase(function.toStdString()); } - void deleteVariableContract(QString var) { + void deleteVariableContract(const QString& var) { mVariableContracts.erase(var); } @@ -313,7 +313,7 @@ public: void setLibraries(const QStringList &libraries); /** Set contract for a function */ - void setFunctionContract(QString function, QString expects); + void setFunctionContract(const QString& function, const QString& expects); /** * @brief Set platform. @@ -350,7 +350,7 @@ public: } /** Set tags for a warning */ - void setWarningTags(std::size_t hash, QString tags); + void setWarningTags(std::size_t hash, const QString& tags); /** Get tags for a warning */ QString getWarningTags(std::size_t hash) const; diff --git a/gui/variablecontractsdialog.cpp b/gui/variablecontractsdialog.cpp index b84d0c285..b73465da8 100644 --- a/gui/variablecontractsdialog.cpp +++ b/gui/variablecontractsdialog.cpp @@ -13,7 +13,7 @@ VariableContractsDialog::VariableContractsDialog(QWidget *parent, QString var) : this->setWindowTitle(mVarName); - auto getMinMax = [](QString var, QString minmax) { + auto getMinMax = [](const QString& var, const QString& minmax) { if (var.indexOf(" " + minmax + ":") < 0) return QString(); int pos1 = var.indexOf(" " + minmax + ":") + 2 + minmax.length(); diff --git a/lib/checkother.cpp b/lib/checkother.cpp index b5db71c20..981cdec77 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1170,7 +1170,7 @@ static int estimateSize(const Type* type, const Settings* settings, const Symbol return cumulatedSize; } -static bool canBeConst(const Variable *var) +static bool canBeConst(const Variable *var, const Settings* settings) { { // check initializer list. If variable is moved from it can't be const. @@ -1214,17 +1214,26 @@ static bool canBeConst(const Variable *var) argNr++; tok3 = tok3->previous(); } - if (!tok3 || tok3->str() != "(" || !tok3->astOperand1() || !tok3->astOperand1()->function()) + if (!tok3 || tok3->str() != "(") return false; - else { - const Variable* argVar = tok3->astOperand1()->function()->getArgumentVar(argNr); + const Token* functionTok = tok3->astOperand1(); + if (!functionTok) + return false; + const Function* tokFunction = functionTok->function(); + if (!tokFunction && functionTok->str() == "." && (functionTok = functionTok->astOperand2())) + tokFunction = functionTok->function(); + if (tokFunction) { + const Variable* argVar = tokFunction->getArgumentVar(argNr); if (!argVar || (!argVar->isConst() && argVar->isReference())) return false; } + else if (!settings->library.isFunctionConst(functionTok)) + return false; } else if (parent->isUnaryOp("&")) { // TODO: check how pointer is used return false; - } else if (parent->isConstOp()) + } else if (parent->isConstOp() || + (parent->astOperand2() && settings->library.isFunctionConst(parent->astOperand2()))) continue; else if (parent->isAssignmentOp()) { if (parent->astOperand1() == tok2) @@ -1290,7 +1299,7 @@ void CheckOther::checkPassByReference() if (!var->scope() || var->scope()->function->hasVirtualSpecifier()) continue; - if (canBeConst(var)) { + if (canBeConst(var, mSettings)) { passedByValueError(var->nameToken(), var->name(), inconclusive); } } diff --git a/test/cfg/qt.cpp b/test/cfg/qt.cpp index e7e01a211..a496704e9 100644 --- a/test/cfg/qt.cpp +++ b/test/cfg/qt.cpp @@ -55,6 +55,21 @@ void QString4() QString qs; } +// cppcheck-suppress passedByValue +bool QString5(QString s) { // #10710 + return s.isEmpty(); +} + +// cppcheck-suppress passedByValue +QStringList QString6(QString s) { + return QStringList{ "*" + s + "*" }; +} + +// cppcheck-suppress passedByValue +bool QString7(QString s, const QString& l) { + return l.startsWith(s); +} + void QByteArray1(QByteArray byteArrayArg) { for (int i = 0; i <= byteArrayArg.size(); ++i) { @@ -297,6 +312,7 @@ QVector::iterator QVector2() return it; } +// cppcheck-suppress passedByValue void duplicateExpression_QString_Compare(QString style) //#8723 { // cppcheck-suppress duplicateExpression diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index ee072d30b..3e06bd92a 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -3261,6 +3261,7 @@ void uninitvar_setbase(void) std::cout << std::setbase(p); } +// cppcheck-suppress passedByValue void uninitvar_find(std::string s) { // testing of size_t find (const string& str, size_t pos = 0) @@ -3341,6 +3342,7 @@ void ignoredReturnValue_abs(int i) std::abs(-199); } +// cppcheck-suppress passedByValue void ignoredReturnValue_string_compare(std::string teststr, std::wstring testwstr) { // cppcheck-suppress ignoredReturnValue