Skip to content

Commit cb5a50c

Browse files
Fix #10710 FN passedByValue with QString (cppcheck-opensource#3696)
1 parent 7b793af commit cb5a50c

8 files changed

Lines changed: 45 additions & 18 deletions

File tree

gui/mainwindow.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1918,15 +1918,15 @@ void MainWindow::editVariableContract(QString var)
19181918
updateVariableContractsTab();
19191919
}
19201920

1921-
void MainWindow::deleteFunctionContract(QString function)
1921+
void MainWindow::deleteFunctionContract(const QString& function)
19221922
{
19231923
if (mProjectFile) {
19241924
mProjectFile->deleteFunctionContract(function);
19251925
mProjectFile->write();
19261926
}
19271927
}
19281928

1929-
void MainWindow::deleteVariableContract(QString var)
1929+
void MainWindow::deleteVariableContract(const QString& var)
19301930
{
19311931
if (mProjectFile) {
19321932
mProjectFile->deleteVariableContract(var);

gui/mainwindow.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,10 @@ protected slots:
234234
void editVariableContract(QString var);
235235

236236
/** Delete contract for function */
237-
void deleteFunctionContract(QString function);
237+
void deleteFunctionContract(const QString& function);
238238

239239
/** Edit constraints for variable */
240-
void deleteVariableContract(QString var);
240+
void deleteVariableContract(const QString& var);
241241

242242
private:
243243

gui/projectfile.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -788,7 +788,7 @@ void ProjectFile::setLibraries(const QStringList &libraries)
788788
mLibraries = libraries;
789789
}
790790

791-
void ProjectFile::setFunctionContract(QString function, QString expects)
791+
void ProjectFile::setFunctionContract(const QString& function, const QString& expects)
792792
{
793793
mFunctionContracts[function.toStdString()] = expects.toStdString();
794794
}
@@ -818,7 +818,7 @@ void ProjectFile::setVSConfigurations(const QStringList &vsConfigs)
818818
mVsConfigurations = vsConfigs;
819819
}
820820

821-
void ProjectFile::setWarningTags(std::size_t hash, QString tags)
821+
void ProjectFile::setWarningTags(std::size_t hash, const QString& tags)
822822
{
823823
if (tags.isEmpty())
824824
mWarningTags.erase(hash);

gui/projectfile.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,15 +236,15 @@ class ProjectFile : public QObject {
236236
return mVariableContracts;
237237
}
238238

239-
void setVariableContracts(QString var, QString min, QString max) {
239+
void setVariableContracts(QString var, const QString& min, const QString& max) {
240240
mVariableContracts[var] = Settings::VariableContracts{min.toStdString(), max.toStdString()};
241241
}
242242

243-
void deleteFunctionContract(QString function) {
243+
void deleteFunctionContract(const QString& function) {
244244
mFunctionContracts.erase(function.toStdString());
245245
}
246246

247-
void deleteVariableContract(QString var) {
247+
void deleteVariableContract(const QString& var) {
248248
mVariableContracts.erase(var);
249249
}
250250

@@ -313,7 +313,7 @@ class ProjectFile : public QObject {
313313
void setLibraries(const QStringList &libraries);
314314

315315
/** Set contract for a function */
316-
void setFunctionContract(QString function, QString expects);
316+
void setFunctionContract(const QString& function, const QString& expects);
317317

318318
/**
319319
* @brief Set platform.
@@ -350,7 +350,7 @@ class ProjectFile : public QObject {
350350
}
351351

352352
/** Set tags for a warning */
353-
void setWarningTags(std::size_t hash, QString tags);
353+
void setWarningTags(std::size_t hash, const QString& tags);
354354

355355
/** Get tags for a warning */
356356
QString getWarningTags(std::size_t hash) const;

gui/variablecontractsdialog.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ VariableContractsDialog::VariableContractsDialog(QWidget *parent, QString var) :
1313

1414
this->setWindowTitle(mVarName);
1515

16-
auto getMinMax = [](QString var, QString minmax) {
16+
auto getMinMax = [](const QString& var, const QString& minmax) {
1717
if (var.indexOf(" " + minmax + ":") < 0)
1818
return QString();
1919
int pos1 = var.indexOf(" " + minmax + ":") + 2 + minmax.length();

lib/checkother.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,7 +1170,7 @@ static int estimateSize(const Type* type, const Settings* settings, const Symbol
11701170
return cumulatedSize;
11711171
}
11721172

1173-
static bool canBeConst(const Variable *var)
1173+
static bool canBeConst(const Variable *var, const Settings* settings)
11741174
{
11751175
{
11761176
// check initializer list. If variable is moved from it can't be const.
@@ -1214,17 +1214,26 @@ static bool canBeConst(const Variable *var)
12141214
argNr++;
12151215
tok3 = tok3->previous();
12161216
}
1217-
if (!tok3 || tok3->str() != "(" || !tok3->astOperand1() || !tok3->astOperand1()->function())
1217+
if (!tok3 || tok3->str() != "(")
12181218
return false;
1219-
else {
1220-
const Variable* argVar = tok3->astOperand1()->function()->getArgumentVar(argNr);
1219+
const Token* functionTok = tok3->astOperand1();
1220+
if (!functionTok)
1221+
return false;
1222+
const Function* tokFunction = functionTok->function();
1223+
if (!tokFunction && functionTok->str() == "." && (functionTok = functionTok->astOperand2()))
1224+
tokFunction = functionTok->function();
1225+
if (tokFunction) {
1226+
const Variable* argVar = tokFunction->getArgumentVar(argNr);
12211227
if (!argVar || (!argVar->isConst() && argVar->isReference()))
12221228
return false;
12231229
}
1230+
else if (!settings->library.isFunctionConst(functionTok))
1231+
return false;
12241232
} else if (parent->isUnaryOp("&")) {
12251233
// TODO: check how pointer is used
12261234
return false;
1227-
} else if (parent->isConstOp())
1235+
} else if (parent->isConstOp() ||
1236+
(parent->astOperand2() && settings->library.isFunctionConst(parent->astOperand2())))
12281237
continue;
12291238
else if (parent->isAssignmentOp()) {
12301239
if (parent->astOperand1() == tok2)
@@ -1290,7 +1299,7 @@ void CheckOther::checkPassByReference()
12901299
if (!var->scope() || var->scope()->function->hasVirtualSpecifier())
12911300
continue;
12921301

1293-
if (canBeConst(var)) {
1302+
if (canBeConst(var, mSettings)) {
12941303
passedByValueError(var->nameToken(), var->name(), inconclusive);
12951304
}
12961305
}

test/cfg/qt.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,21 @@ void QString4()
5555
QString qs;
5656
}
5757

58+
// cppcheck-suppress passedByValue
59+
bool QString5(QString s) { // #10710
60+
return s.isEmpty();
61+
}
62+
63+
// cppcheck-suppress passedByValue
64+
QStringList QString6(QString s) {
65+
return QStringList{ "*" + s + "*" };
66+
}
67+
68+
// cppcheck-suppress passedByValue
69+
bool QString7(QString s, const QString& l) {
70+
return l.startsWith(s);
71+
}
72+
5873
void QByteArray1(QByteArray byteArrayArg)
5974
{
6075
for (int i = 0; i <= byteArrayArg.size(); ++i) {
@@ -297,6 +312,7 @@ QVector<int>::iterator QVector2()
297312
return it;
298313
}
299314

315+
// cppcheck-suppress passedByValue
300316
void duplicateExpression_QString_Compare(QString style) //#8723
301317
{
302318
// cppcheck-suppress duplicateExpression

test/cfg/std.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3261,6 +3261,7 @@ void uninitvar_setbase(void)
32613261
std::cout << std::setbase(p);
32623262
}
32633263

3264+
// cppcheck-suppress passedByValue
32643265
void uninitvar_find(std::string s)
32653266
{
32663267
// testing of size_t find (const string& str, size_t pos = 0)
@@ -3341,6 +3342,7 @@ void ignoredReturnValue_abs(int i)
33413342
std::abs(-199);
33423343
}
33433344

3345+
// cppcheck-suppress passedByValue
33443346
void ignoredReturnValue_string_compare(std::string teststr, std::wstring testwstr)
33453347
{
33463348
// cppcheck-suppress ignoredReturnValue

0 commit comments

Comments
 (0)