From 25678a9fafb8cb1e06d7f07f6ecb13979710bb38 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sat, 20 Feb 2021 12:52:39 +0100 Subject: [PATCH] Refactorized CheckStl::string_c_str(), fixing some false negatives by supporting member functions and overloads Merged from LCppC. --- lib/checkstl.cpp | 44 ++++++++++++++++++++------------------------ test/teststl.cpp | 9 +++++---- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 19c3cc604..b7b702870 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1804,21 +1804,15 @@ void CheckStl::string_c_str() const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase(); // Find all functions that take std::string as argument - std::multimap c_strFuncParam; + std::multimap c_strFuncParam; if (printPerformance) { for (const Scope &scope : symbolDatabase->scopeList) { 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; - 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) { numpar++; 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()) && tok->next()->variable() && tok->next()->variable()->isStlStringType()) { string_c_strThrowError(tok); - } else if (Token::Match(tok, "[;{}] %name% = %var% . str ( ) . c_str|data ( ) ;")) { - const Variable* var = tok->next()->variable(); - const Variable* var2 = tok->tokAt(3)->variable(); - if (var && var->isPointer() && var2 && var2->isStlType(stl_string_stream)) - string_c_strError(tok); - } else if (Token::Match(tok, "[;{}] %var% = %name% (") && - Token::Match(tok->linkAt(4), ") . c_str|data ( ) ;") && - tok->tokAt(3)->function() && Token::Match(tok->tokAt(3)->function()->retDef, "std :: string|wstring %name%")) { - const Variable* var = tok->next()->variable(); - if (var && var->isPointer()) - string_c_strError(tok); - } else if (printPerformance && Token::Match(tok, "%name% ( !!)") && c_strFuncParam.find(tok->str()) != c_strFuncParam.end() && - !Token::Match(tok->previous(), "::|.") && tok->varId() == 0 && tok->str() != scope.className) { // calling function. TODO: Add support for member functions - const std::pair::const_iterator, std::multimap::const_iterator> range = c_strFuncParam.equal_range(tok->str()); - for (std::multimap::const_iterator i = range.first; i != range.second; ++i) { + } else if (tok->variable() && tok->strAt(1) == "=") { + if (Token::Match(tok->tokAt(2), "%var% . str ( ) . c_str|data ( ) ;")) { + const Variable* var = tok->variable(); + const Variable* var2 = tok->tokAt(2)->variable(); + if (var && var->isPointer() && var2 && var2->isStlType(stl_string_stream)) + string_c_strError(tok); + } else if (Token::Match(tok->tokAt(2), "%name% (") && + Token::Match(tok->linkAt(3), ") . c_str|data ( ) ;") && + tok->tokAt(2)->function() && Token::Match(tok->tokAt(2)->function()->retDef, "std :: string|wstring %name%")) { + const Variable* var = tok->variable(); + if (var && var->isPointer()) + string_c_strError(tok); + } + } else if (printPerformance && tok->function() && Token::Match(tok, "%name% ( !!)") && c_strFuncParam.find(tok->function()) != c_strFuncParam.end() && + tok->str() != scope.className) { + const std::pair::const_iterator, std::multimap::const_iterator> range = c_strFuncParam.equal_range(tok->function()); + for (std::multimap::const_iterator i = range.first; i != range.second; ++i) { if (i->second == 0) 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* - 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; const Token* tok2 = tok->next(); diff --git a/test/teststl.cpp b/test/teststl.cpp index 3cf96e6f4..64c1f7fea 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -3214,7 +3214,7 @@ private: ASSERT_EQUALS("", errout.str()); 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 Foo4(std::string str, const std::string& str) {}\n" "void Bar() {\n" @@ -3261,9 +3261,10 @@ private: " Foo::sfunc(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" - "[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", - "", errout.str()); + 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 no. 1 is slow and redundant.\n" + "[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" "void Bar(std::string filename) {\n"