walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Refactorized CheckStl::string_c_str(), fixing some false negatives by supporting member functions and overloads 

Merged from LCppC.
This commit is contained in:
PKEuS 2021-02-20 12:52:39 +01:00 committed by Daniel Marjamäki
parent 423d7dbc3c
commit 25678a9faf
2 changed files with 25 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
lib/checkstl.cpp
View File

@ -1804,21 +1804,15 @@ void CheckStl::string_c_str()
const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase(); const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase();
// Find all functions that take std::string as argument // Find all functions that take std::string as argument
std::multimap<std::string, int> c_strFuncParam; std::multimap<const Function*, int> c_strFuncParam;
if (printPerformance) { if (printPerformance) {
for (const Scope &scope : symbolDatabase->scopeList) { for (const Scope &scope : symbolDatabase->scopeList) {
for (const Function &func : scope.functionList) { for (const Function &func : scope.functionList) {
if (c_strFuncParam.erase(func.tokenDef->str()) != 0) { // Check if function with this name was already found
c_strFuncParam.insert(std::make_pair(func.tokenDef->str(), 0)); // Disable, because there are overloads. TODO: Handle overloads
continue;
}
int numpar = 0; int numpar = 0;
c_strFuncParam.insert(std::make_pair(func.tokenDef->str(), numpar)); // Insert function as dummy, to indicate that there is at least one function with that name
for (const Variable &var : func.argumentList) { for (const Variable &var : func.argumentList) {
numpar++; numpar++;
if (var.isStlStringType() && (!var.isReference() || var.isConst())) if (var.isStlStringType() && (!var.isReference() || var.isConst()))
c_strFuncParam.insert(std::make_pair(func.tokenDef->str(), numpar)); c_strFuncParam.insert(std::make_pair(&func, numpar));
} }
} }
} }
@ -1842,21 +1836,23 @@ void CheckStl::string_c_str()
if (Token::Match(tok, "throw %var% . c_str|data ( ) ;") && isLocal(tok->next()) && if (Token::Match(tok, "throw %var% . c_str|data ( ) ;") && isLocal(tok->next()) &&
tok->next()->variable() && tok->next()->variable()->isStlStringType()) { tok->next()->variable() && tok->next()->variable()->isStlStringType()) {
string_c_strThrowError(tok); string_c_strThrowError(tok);
} else if (Token::Match(tok, "[;{}] %name% = %var% . str ( ) . c_str|data ( ) ;")) { } else if (tok->variable() && tok->strAt(1) == "=") {
const Variable* var = tok->next()->variable(); if (Token::Match(tok->tokAt(2), "%var% . str ( ) . c_str|data ( ) ;")) {
const Variable* var2 = tok->tokAt(3)->variable(); const Variable* var = tok->variable();
if (var && var->isPointer() && var2 && var2->isStlType(stl_string_stream)) const Variable* var2 = tok->tokAt(2)->variable();
string_c_strError(tok); if (var && var->isPointer() && var2 && var2->isStlType(stl_string_stream))
} else if (Token::Match(tok, "[;{}] %var% = %name% (") && string_c_strError(tok);
Token::Match(tok->linkAt(4), ") . c_str|data ( ) ;") && } else if (Token::Match(tok->tokAt(2), "%name% (") &&
tok->tokAt(3)->function() && Token::Match(tok->tokAt(3)->function()->retDef, "std :: string|wstring %name%")) { Token::Match(tok->linkAt(3), ") . c_str|data ( ) ;") &&
const Variable* var = tok->next()->variable(); tok->tokAt(2)->function() && Token::Match(tok->tokAt(2)->function()->retDef, "std :: string|wstring %name%")) {
if (var && var->isPointer()) const Variable* var = tok->variable();
string_c_strError(tok); if (var && var->isPointer())
} else if (printPerformance && Token::Match(tok, "%name% ( !!)") && c_strFuncParam.find(tok->str()) != c_strFuncParam.end() && string_c_strError(tok);
!Token::Match(tok->previous(), "::|.") && tok->varId() == 0 && tok->str() != scope.className) { // calling function. TODO: Add support for member functions }
const std::pair<std::multimap<std::string, int>::const_iterator, std::multimap<std::string, int>::const_iterator> range = c_strFuncParam.equal_range(tok->str()); } else if (printPerformance && tok->function() && Token::Match(tok, "%name% ( !!)") && c_strFuncParam.find(tok->function()) != c_strFuncParam.end() &&
for (std::multimap<std::string, int>::const_iterator i = range.first; i != range.second; ++i) { tok->str() != scope.className) {
const std::pair<std::multimap<const Function*, int>::const_iterator, std::multimap<const Function*, int>::const_iterator> range = c_strFuncParam.equal_range(tok->function());
for (std::multimap<const Function*, int>::const_iterator i = range.first; i != range.second; ++i) {
if (i->second == 0) if (i->second == 0)
continue; continue;
@ -1889,7 +1885,7 @@ void CheckStl::string_c_str()
} }
// Using c_str() to get the return value is only dangerous if the function returns a char* // Using c_str() to get the return value is only dangerous if the function returns a char*
if ((returnType == charPtr || (printPerformance && (returnType == stdString || returnType == stdStringConstRef))) && tok->str() == "return") { else if ((returnType == charPtr || (printPerformance && (returnType == stdString || returnType == stdStringConstRef))) && tok->str() == "return") {
bool err = false; bool err = false;
const Token* tok2 = tok->next(); const Token* tok2 = tok->next();

9
test/teststl.cpp
View File

@ -3214,7 +3214,7 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void Foo1(const std::string& str) {}\n" check("void Foo1(const std::string& str) {}\n"
"void Foo2(char* c, const std::string str) {}\n" "void Foo2(const char* c, const std::string str) {}\n"
"void Foo3(std::string& rstr) {}\n" "void Foo3(std::string& rstr) {}\n"
"void Foo4(std::string str, const std::string& str) {}\n" "void Foo4(std::string str, const std::string& str) {}\n"
"void Bar() {\n" "void Bar() {\n"
@ -3261,9 +3261,10 @@ private:
" Foo::sfunc(str.c_str());\n" " Foo::sfunc(str.c_str());\n"
" foo.func(str.c_str());\n" " foo.func(str.c_str());\n"
"}"); "}");
TODO_ASSERT_EQUALS("[test.cpp:9]: (performance) Passing the result of c_str() to a function that takes std::string as argument 1 is slow and redundant.\n" ASSERT_EQUALS("[test.cpp:9]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
"[test.cpp:10]: (performance) Passing the result of c_str() to a function that takes std::string as argument 1 is slow and redundant.\n", "[test.cpp:10]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
"", errout.str()); "[test.cpp:11]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n",
errout.str());
check("void svgFile(const std::string &content, const std::string &fileName, const double end = 1000., const double start = 0.);\n" check("void svgFile(const std::string &content, const std::string &fileName, const double end = 1000., const double start = 0.);\n"
"void Bar(std::string filename) {\n" "void Bar(std::string filename) {\n"