Skip to content

Commit b7d8cd6

Browse files
PKEuSPKEuS
committed
Fixed false negatives in CheckStl::string_c_str():
- Support more complex patterns (danmar#7385) - Use same logic for string_c_strReturn() as for string_c_strError()
1 parent 88449a7 commit b7d8cd6

File tree

2 files changed

+65
-31
lines changed

2 files changed

+65
-31
lines changed

‎lib/checkstl.cpp‎

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,26 +1019,19 @@ void CheckStl::string_c_str()
10191019
}
10201020

10211021
// Using c_str() to get the return value is only dangerous if the function returns a char*
1022-
if (returnType == charPtr) {
1023-
if (Token::Match(tok, "return %var% . c_str|data ( ) ;") && isLocal(tok->next()) &&
1024-
tok->next()->variable() && tok->next()->variable()->isStlStringType()) {
1025-
string_c_strError(tok);
1026-
} else if (Token::Match(tok, "return %var% . str ( ) . c_str|data ( ) ;") && isLocal(tok->next()) &&
1027-
tok->next()->variable() && tok->next()->variable()->isStlType(stl_string_stream)) {
1028-
string_c_strError(tok);
1029-
} else if (Token::Match(tok, "return std :: string|wstring (") &&
1030-
Token::Match(tok->linkAt(4), ") . c_str|data ( ) ;")) {
1031-
string_c_strError(tok);
1032-
} else if (Token::Match(tok, "return %name% (") && Token::Match(tok->linkAt(2), ") . c_str|data ( ) ;")) {
1033-
const Function* func = tok->next()->function();
1034-
if (func && Token::Match(func->tokenDef->tokAt(-3), "std :: string|wstring"))
1035-
string_c_strError(tok);
1036-
} else if (Token::simpleMatch(tok, "return (") &&
1037-
Token::Match(tok->next()->link(), ") . c_str|data ( ) ;")) {
1022+
if ((returnType == charPtr || (printPerformance && (returnType == stdString || returnType == stdStringConstRef))) && tok->str() == "return") {
1023+
bool err = false;
1024+
1025+
const Token* tok2 = tok->next();
1026+
if (Token::Match(tok2, "std :: string|wstring (") &&
1027+
Token::Match(tok2->linkAt(3), ") . c_str|data ( ) ;")) {
1028+
err = true;
1029+
} else if (Token::simpleMatch(tok2, "(") &&
1030+
Token::Match(tok2->link(), ") . c_str|data ( ) ;")) {
10381031
// Check for "+ localvar" or "+ std::string(" inside the bracket
10391032
bool is_implicit_std_string = printInconclusive;
1040-
const Token *search_end = tok->next()->link();
1041-
for (const Token *search_tok = tok->tokAt(2); search_tok != search_end; search_tok = search_tok->next()) {
1033+
const Token *search_end = tok2->link();
1034+
for (const Token *search_tok = tok2->next(); search_tok != search_end; search_tok = search_tok->next()) {
10421035
if (Token::Match(search_tok, "+ %var%") && isLocal(search_tok->next()) &&
10431036
search_tok->next()->variable() && search_tok->next()->variable()->isStlStringType()) {
10441037
is_implicit_std_string = true;
@@ -1050,19 +1043,43 @@ void CheckStl::string_c_str()
10501043
}
10511044

10521045
if (is_implicit_std_string)
1053-
string_c_strError(tok);
1046+
err = true;
10541047
}
1055-
}
1056-
// Using c_str() to get the return value is redundant if the function returns std::string or const std::string&.
1057-
else if (printPerformance && (returnType == stdString || returnType == stdStringConstRef)) {
1058-
if (tok->str() == "return") {
1059-
const Token* tok2 = Token::findsimplematch(tok->next(), ";");
1060-
if (Token::Match(tok2->tokAt(-4), ". c_str|data ( )")) {
1061-
tok2 = tok2->tokAt(-5);
1062-
if (tok2->variable() && tok2->variable()->isStlStringType()) { // return var.c_str();
1063-
string_c_strReturn(tok);
1064-
}
1065-
}
1048+
1049+
bool local = false;
1050+
const Variable* lastVar = nullptr;
1051+
const Function* lastFunc = nullptr;
1052+
bool funcStr = false;
1053+
if (Token::Match(tok2, "%var% .")) {
1054+
local = isLocal(tok2);
1055+
}
1056+
while (tok2) {
1057+
if (Token::Match(tok2, "%var% .|::")) {
1058+
lastVar = tok2->variable();
1059+
tok2 = tok2->tokAt(2);
1060+
} else if (Token::Match(tok2, "%name% (") && Token::simpleMatch(tok2->linkAt(1), ") .")) {
1061+
lastFunc = tok2->function();
1062+
local = false;
1063+
funcStr = tok2->str() == "str";
1064+
tok2 = tok2->linkAt(1)->tokAt(2);
1065+
} else
1066+
break;
1067+
}
1068+
1069+
if (Token::Match(tok2, "c_str|data ( ) ;")) {
1070+
if (local && lastVar && lastVar->isStlStringType())
1071+
err = true;
1072+
else if (funcStr && lastVar && lastVar->isStlType(stl_string_stream))
1073+
err = true;
1074+
else if (lastFunc && Token::Match(lastFunc->tokenDef->tokAt(-3), "std :: string|wstring"))
1075+
err = true;
1076+
}
1077+
1078+
if (err) {
1079+
if (returnType == charPtr)
1080+
string_c_strError(tok);
1081+
else
1082+
string_c_strReturn(tok);
10661083
}
10671084
}
10681085
}

‎test/teststl.cpp‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2093,7 +2093,6 @@ class TestStl : public TestFixture {
20932093
" std::string errmsg;\n"
20942094
" return errmsg.c_str();\n"
20952095
"}");
2096-
20972096
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());
20982097

20992098
check("const char *get_msg() {\n"
@@ -2142,6 +2141,15 @@ class TestStl : public TestFixture {
21422141
"}");
21432142
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());
21442143

2144+
check("class Foo {\n"
2145+
" std::string GetVal() const;\n"
2146+
"};\n"
2147+
"const char *f() {\n"
2148+
" Foo f;\n"
2149+
" return f.GetVal().c_str();\n"
2150+
"}");
2151+
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());
2152+
21452153
check("const char* foo() {\n"
21462154
" static std::string text;\n"
21472155
" text = \"hello world\n\";\n"
@@ -2162,6 +2170,15 @@ class TestStl : public TestFixture {
21622170
"}");
21632171
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());
21642172

2173+
check("class Foo {\n"
2174+
" std::string GetVal() const;\n"
2175+
"};\n"
2176+
"std::string f() {\n"
2177+
" Foo f;\n"
2178+
" return f.GetVal().c_str();\n"
2179+
"}");
2180+
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());
2181+
21652182
check("std::string get_msg() {\n"
21662183
" std::string errmsg;\n"
21672184
" return errmsg;\n"

0 commit comments

Comments
 (0)