diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 9d43ced6e..fe3ec215e 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1019,26 +1019,19 @@ 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) { - if (Token::Match(tok, "return %var% . c_str|data ( ) ;") && isLocal(tok->next()) && - tok->next()->variable() && tok->next()->variable()->isStlStringType()) { - string_c_strError(tok); - } else if (Token::Match(tok, "return %var% . str ( ) . c_str|data ( ) ;") && isLocal(tok->next()) && - tok->next()->variable() && tok->next()->variable()->isStlType(stl_string_stream)) { - string_c_strError(tok); - } else if (Token::Match(tok, "return std :: string|wstring (") && - Token::Match(tok->linkAt(4), ") . c_str|data ( ) ;")) { - string_c_strError(tok); - } else if (Token::Match(tok, "return %name% (") && Token::Match(tok->linkAt(2), ") . c_str|data ( ) ;")) { - const Function* func = tok->next()->function(); - if (func && Token::Match(func->tokenDef->tokAt(-3), "std :: string|wstring")) - string_c_strError(tok); - } else if (Token::simpleMatch(tok, "return (") && - Token::Match(tok->next()->link(), ") . c_str|data ( ) ;")) { + if ((returnType == charPtr || (printPerformance && (returnType == stdString || returnType == stdStringConstRef))) && tok->str() == "return") { + bool err = false; + + const Token* tok2 = tok->next(); + if (Token::Match(tok2, "std :: string|wstring (") && + Token::Match(tok2->linkAt(3), ") . c_str|data ( ) ;")) { + err = true; + } else if (Token::simpleMatch(tok2, "(") && + Token::Match(tok2->link(), ") . c_str|data ( ) ;")) { // Check for "+ localvar" or "+ std::string(" inside the bracket bool is_implicit_std_string = printInconclusive; - const Token *search_end = tok->next()->link(); - for (const Token *search_tok = tok->tokAt(2); search_tok != search_end; search_tok = search_tok->next()) { + const Token *search_end = tok2->link(); + for (const Token *search_tok = tok2->next(); search_tok != search_end; search_tok = search_tok->next()) { if (Token::Match(search_tok, "+ %var%") && isLocal(search_tok->next()) && search_tok->next()->variable() && search_tok->next()->variable()->isStlStringType()) { is_implicit_std_string = true; @@ -1050,19 +1043,43 @@ void CheckStl::string_c_str() } if (is_implicit_std_string) - string_c_strError(tok); + err = true; } - } - // Using c_str() to get the return value is redundant if the function returns std::string or const std::string&. - else if (printPerformance && (returnType == stdString || returnType == stdStringConstRef)) { - if (tok->str() == "return") { - const Token* tok2 = Token::findsimplematch(tok->next(), ";"); - if (Token::Match(tok2->tokAt(-4), ". c_str|data ( )")) { - tok2 = tok2->tokAt(-5); - if (tok2->variable() && tok2->variable()->isStlStringType()) { // return var.c_str(); - string_c_strReturn(tok); - } - } + + bool local = false; + const Variable* lastVar = nullptr; + const Function* lastFunc = nullptr; + bool funcStr = false; + if (Token::Match(tok2, "%var% .")) { + local = isLocal(tok2); + } + while (tok2) { + if (Token::Match(tok2, "%var% .|::")) { + lastVar = tok2->variable(); + tok2 = tok2->tokAt(2); + } else if (Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") .")) { + lastFunc = tok2->function(); + local = false; + funcStr = tok2->str() == "str"; + tok2 = tok2->linkAt(1)->tokAt(2); + } else + break; + } + + if (Token::Match(tok2, "c_str|data ( ) ;")) { + if (local && lastVar && lastVar->isStlStringType()) + err = true; + else if (funcStr && lastVar && lastVar->isStlType(stl_string_stream)) + err = true; + else if (lastFunc && Token::Match(lastFunc->tokenDef->tokAt(-3), "std :: string|wstring")) + err = true; + } + + if (err) { + if (returnType == charPtr) + string_c_strError(tok); + else + string_c_strReturn(tok); } } } diff --git a/test/teststl.cpp b/test/teststl.cpp index 997711c3e..d6aad2d9c 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -2093,7 +2093,6 @@ private: " std::string errmsg;\n" " return errmsg.c_str();\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous usage of c_str(). The value returned by c_str() is invalid after this call.\n", errout.str()); check("const char *get_msg() {\n" @@ -2142,6 +2141,15 @@ private: "}"); ASSERT_EQUALS("[test.cpp:6]: (error) Dangerous usage of c_str(). The value returned by c_str() is invalid after this call.\n", errout.str()); + check("class Foo {\n" + " std::string GetVal() const;\n" + "};\n" + "const char *f() {\n" + " Foo f;\n" + " return f.GetVal().c_str();\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (error) Dangerous usage of c_str(). The value returned by c_str() is invalid after this call.\n", errout.str()); + check("const char* foo() {\n" " static std::string text;\n" " text = \"hello world\n\";\n" @@ -2162,6 +2170,15 @@ private: "}"); ASSERT_EQUALS("[test.cpp:3]: (performance) Returning the result of c_str() in a function that returns std::string is slow and redundant.\n", errout.str()); + check("class Foo {\n" + " std::string GetVal() const;\n" + "};\n" + "std::string f() {\n" + " Foo f;\n" + " return f.GetVal().c_str();\n" + "}"); + ASSERT_EQUALS("[test.cpp:6]: (performance) Returning the result of c_str() in a function that returns std::string is slow and redundant.\n", errout.str()); + check("std::string get_msg() {\n" " std::string errmsg;\n" " return errmsg;\n"