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

Fixed false negatives in CheckStl::string_c_str(): 

- Support more complex patterns (#7385)
- Use same logic for string_c_strReturn() as for string_c_strError()
This commit is contained in:
PKEuS 2016-05-04 11:10:12 +02:00
parent 88449a78c5
commit b7d8cd69f6
2 changed files with 65 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
lib/checkstl.cpp
View File

@ -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* // Using c_str() to get the return value is only dangerous if the function returns a char*
if (returnType == charPtr) { if ((returnType == charPtr || (printPerformance && (returnType == stdString || returnType == stdStringConstRef))) && tok->str() == "return") {
if (Token::Match(tok, "return %var% . c_str|data ( ) ;") && isLocal(tok->next()) && bool err = false;
tok->next()->variable() && tok->next()->variable()->isStlStringType()) {
string_c_strError(tok); const Token* tok2 = tok->next();
} else if (Token::Match(tok, "return %var% . str ( ) . c_str|data ( ) ;") && isLocal(tok->next()) && if (Token::Match(tok2, "std :: string|wstring (") &&
tok->next()->variable() && tok->next()->variable()->isStlType(stl_string_stream)) { Token::Match(tok2->linkAt(3), ") . c_str|data ( ) ;")) {
string_c_strError(tok); err = true;
} else if (Token::Match(tok, "return std :: string|wstring (") && } else if (Token::simpleMatch(tok2, "(") &&
Token::Match(tok->linkAt(4), ") . c_str|data ( ) ;")) { Token::Match(tok2->link(), ") . 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 ( ) ;")) {
// Check for "+ localvar" or "+ std::string(" inside the bracket // Check for "+ localvar" or "+ std::string(" inside the bracket
bool is_implicit_std_string = printInconclusive; bool is_implicit_std_string = printInconclusive;
const Token *search_end = tok->next()->link(); const Token *search_end = tok2->link();
for (const Token *search_tok = tok->tokAt(2); search_tok != search_end; search_tok = search_tok->next()) { 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()) && if (Token::Match(search_tok, "+ %var%") && isLocal(search_tok->next()) &&
search_tok->next()->variable() && search_tok->next()->variable()->isStlStringType()) { search_tok->next()->variable() && search_tok->next()->variable()->isStlStringType()) {
is_implicit_std_string = true; is_implicit_std_string = true;
@ -1050,19 +1043,43 @@ void CheckStl::string_c_str()
} }
if (is_implicit_std_string) 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&. bool local = false;
else if (printPerformance && (returnType == stdString || returnType == stdStringConstRef)) { const Variable* lastVar = nullptr;
if (tok->str() == "return") { const Function* lastFunc = nullptr;
const Token* tok2 = Token::findsimplematch(tok->next(), ";"); bool funcStr = false;
if (Token::Match(tok2->tokAt(-4), ". c_str|data ( )")) { if (Token::Match(tok2, "%var% .")) {
tok2 = tok2->tokAt(-5); local = isLocal(tok2);
if (tok2->variable() && tok2->variable()->isStlStringType()) { // return var.c_str(); }
string_c_strReturn(tok); 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);
} }
} }
} }

19
test/teststl.cpp
View File

@ -2093,7 +2093,6 @@ private:
" std::string errmsg;\n" " std::string errmsg;\n"
" return errmsg.c_str();\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()); 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" 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()); 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" check("const char* foo() {\n"
" static std::string text;\n" " static std::string text;\n"
" text = \"hello world\n\";\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()); 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" check("std::string get_msg() {\n"
" std::string errmsg;\n" " std::string errmsg;\n"
" return errmsg;\n" " return errmsg;\n"