Skip to content

Commit a34d2eb

Browse files
lucasmrodlucasmrod
committed
Fixed cppcheck-opensource#4938: (.empty() method false positive for non-STL classes)
1 parent d6e3b3d commit a34d2eb

2 files changed

Lines changed: 58 additions & 20 deletions

File tree

‎lib/checkstl.cpp‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,6 +1132,15 @@ static bool isLocal(const Token *tok)
11321132

11331133
void CheckStl::string_c_str()
11341134
{
1135+
// THIS ARRAY MUST BE ORDERED ALPHABETICALLY
1136+
static const char* const stl_string[] = {
1137+
"string", "u16string", "u32string", "wstring"
1138+
};
1139+
// THIS ARRAY MUST BE ORDERED ALPHABETICALLY
1140+
static const char* const stl_string_stream[] = {
1141+
"istringstream", "ostringstream", "stringstream", "wstringstream"
1142+
};
1143+
11351144
const SymbolDatabase* symbolDatabase = _tokenizer->getSymbolDatabase();
11361145

11371146
// Find all functions that take std::string as argument
@@ -1170,11 +1179,13 @@ void CheckStl::string_c_str()
11701179

11711180
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
11721181
// Invalid usage..
1173-
if (Token::Match(tok, "throw %var% . c_str ( ) ;") && isLocal(tok->next())) {
1182+
if (Token::Match(tok, "throw %var% . c_str ( ) ;") && isLocal(tok->next()) &&
1183+
tok->next()->variable() && tok->next()->variable()->isStlType(stl_string)) {
11741184
string_c_strThrowError(tok);
11751185
} else if (Token::Match(tok, "[;{}] %var% = %var% . str ( ) . c_str ( ) ;")) {
11761186
const Variable* var = tok->next()->variable();
1177-
if (var && var->isPointer())
1187+
const Variable* var2 = tok->tokAt(3)->variable();
1188+
if (var && var->isPointer() && var2 && var2->isStlType(stl_string_stream))
11781189
string_c_strError(tok);
11791190
} else if (Token::Match(tok, "[;{}] %var% = %var% (") &&
11801191
Token::simpleMatch(tok->linkAt(4), ") . c_str ( ) ;") &&
@@ -1203,17 +1214,25 @@ void CheckStl::string_c_str()
12031214
tok2 = tok2->previous();
12041215
if (tok2 && Token::simpleMatch(tok2->tokAt(-4), ". c_str ( )")) {
12051216
const Variable* var = tok2->tokAt(-5)->variable();
1206-
if (var && var->isStlType())
1217+
if (var && var->isStlType(stl_string)) {
12071218
string_c_strParam(tok, i->second);
1219+
} else if (Token::Match(tok2->tokAt(-9), "%var% . str ( )")) { // Check ss.str().c_str() as parameter
1220+
const Variable* ssVar = tok2->tokAt(-9)->variable();
1221+
if (ssVar && ssVar->isStlType(stl_string_stream))
1222+
string_c_strParam(tok, i->second);
1223+
}
1224+
12081225
}
12091226
}
12101227
}
12111228

12121229
// Using c_str() to get the return value is only dangerous if the function returns a char*
12131230
if (returnType == charPtr) {
1214-
if (Token::Match(tok, "return %var% . c_str ( ) ;") && isLocal(tok->next())) {
1231+
if (Token::Match(tok, "return %var% . c_str ( ) ;") && isLocal(tok->next()) &&
1232+
tok->next()->variable() && tok->next()->variable()->isStlType(stl_string)) {
12151233
string_c_strError(tok);
1216-
} else if (Token::Match(tok, "return %var% . str ( ) . c_str ( ) ;") && isLocal(tok->next())) {
1234+
} else if (Token::Match(tok, "return %var% . str ( ) . c_str ( ) ;") && isLocal(tok->next()) &&
1235+
tok->next()->variable() && tok->next()->variable()->isStlType(stl_string_stream)) {
12171236
string_c_strError(tok);
12181237
} else if (Token::Match(tok, "return std :: string|wstring (") &&
12191238
Token::simpleMatch(tok->linkAt(4), ") . c_str ( ) ;")) {
@@ -1228,7 +1247,8 @@ void CheckStl::string_c_str()
12281247
bool is_implicit_std_string = _settings->inconclusive;
12291248
const Token *search_end = tok->next()->link();
12301249
for (const Token *search_tok = tok->tokAt(2); search_tok != search_end; search_tok = search_tok->next()) {
1231-
if (Token::Match(search_tok, "+ %var%") && isLocal(search_tok->next())) {
1250+
if (Token::Match(search_tok, "+ %var%") && isLocal(search_tok->next()) &&
1251+
search_tok->next()->variable() && search_tok->next()->variable()->isStlType(stl_string)) {
12321252
is_implicit_std_string = true;
12331253
break;
12341254
} else if (Token::Match(search_tok, "+ std :: string|wstring (")) {
@@ -1247,12 +1267,7 @@ void CheckStl::string_c_str()
12471267
const Token* tok2 = Token::findsimplematch(tok->next(), ";");
12481268
if (Token::simpleMatch(tok2->tokAt(-4), ". c_str ( )")) {
12491269
tok2 = tok2->tokAt(-5);
1250-
if (tok2->isName()) { // return var.c_str(); => check if var is a std type
1251-
const Variable *var = tok2->variable();
1252-
if (var && var->isStlType())
1253-
string_c_strReturn(tok);
1254-
} else {
1255-
// TODO: determine if a error should be written or not
1270+
if (tok2->variable() && tok2->variable()->isStlType(stl_string)) { // return var.c_str();
12561271
string_c_strReturn(tok);
12571272
}
12581273
}
@@ -1387,6 +1402,14 @@ void CheckStl::autoPointerArrayError(const Token *tok)
13871402

13881403
void CheckStl::uselessCalls()
13891404
{
1405+
// THIS ARRAY MUST BE ORDERED ALPHABETICALLY
1406+
static const char* const stl_containers_with_empty_and_clear[] = {
1407+
"deque", "forward_list", "list",
1408+
"map", "multimap", "multiset", "set", "string",
1409+
"unordered_map", "unordered_multimap", "unordered_multiset",
1410+
"unordered_set", "vector", "wstring"
1411+
};
1412+
13901413
bool performance = _settings->isEnabled("performance");
13911414
bool warning = _settings->isEnabled("warning");
13921415
if (!performance && !warning)
@@ -1411,7 +1434,8 @@ void CheckStl::uselessCalls()
14111434
uselessCallsSubstrError(tok, false);
14121435
} else if (Token::simpleMatch(tok->linkAt(2)->tokAt(-2), ", 0 )"))
14131436
uselessCallsSubstrError(tok, true);
1414-
} else if (Token::Match(tok, "[{};] %var% . empty ( ) ;") && warning)
1437+
} else if (Token::Match(tok, "[{};] %var% . empty ( ) ;") && warning &&
1438+
tok->next()->variable() && tok->next()->variable()->isStlType(stl_containers_with_empty_and_clear))
14151439
uselessCallsEmptyError(tok->next());
14161440
else if (Token::Match(tok, "[{};] std :: remove|remove_if|unique (") && tok->tokAt(5)->nextArgument())
14171441
uselessCallsRemoveError(tok->next(), tok->strAt(3));

‎test/teststl.cpp‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,13 +1922,13 @@ class TestStl : public TestFixture {
19221922
"}");
19231923
ASSERT_EQUALS("", errout.str());
19241924

1925-
19261925
check("void Foo1(const std::string& str) {}\n"
19271926
"void Foo2(char* c, const std::string str) {}\n"
19281927
"void Foo3(std::string& rstr) {}\n"
19291928
"void Foo4(std::string str, const std::string& str) {}\n"
19301929
"void Bar() {\n"
19311930
" std::string str = \"bar\";\n"
1931+
" std::stringstream ss(\"foo\");\n"
19321932
" Foo1(str);\n"
19331933
" Foo1(str.c_str());\n"
19341934
" Foo2(str.c_str(), str);\n"
@@ -1937,14 +1937,17 @@ class TestStl : public TestFixture {
19371937
" Foo4(str, str);\n"
19381938
" Foo4(str.c_str(), str);\n"
19391939
" Foo4(str, str.c_str());\n"
1940+
" Foo4(ss.str(), ss.str().c_str());\n"
19401941
" Foo4(str.c_str(), str.c_str());\n"
19411942
"}");
1942-
ASSERT_EQUALS("[test.cpp:8]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
1943-
"[test.cpp:10]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n"
1944-
"[test.cpp:13]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
1945-
"[test.cpp:14]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n"
1946-
"[test.cpp:15]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
1947-
"[test.cpp:15]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n", errout.str());
1943+
1944+
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"
1945+
"[test.cpp:11]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n"
1946+
"[test.cpp:14]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
1947+
"[test.cpp:15]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n"
1948+
"[test.cpp:16]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n"
1949+
"[test.cpp:17]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 1 is slow and redundant.\n"
1950+
"[test.cpp:17]: (performance) Passing the result of c_str() to a function that takes std::string as argument no. 2 is slow and redundant.\n", errout.str());
19481951

19491952
check("void Foo1(const std::string& str) {}\n"
19501953
"void Foo2(char* c, const std::string str) {}\n"
@@ -2026,6 +2029,11 @@ class TestStl : public TestFixture {
20262029
" a(s.c_str());\n"
20272030
"}");
20282031
ASSERT_EQUALS("", errout.str());
2032+
2033+
check("std::string Format(const char * name) {\n" // #4938
2034+
" return String::Format(\"%s:\", name).c_str();\n"
2035+
"}");
2036+
ASSERT_EQUALS("", errout.str());
20292037
}
20302038

20312039
void autoPointer() {
@@ -2233,6 +2241,12 @@ class TestStl : public TestFixture {
22332241
"}");
22342242
ASSERT_EQUALS("[test.cpp:2]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?\n", errout.str());
22352243

2244+
check("void f() {\n" // #4938
2245+
" OdString str;\n"
2246+
" str.empty();\n"
2247+
"}");
2248+
ASSERT_EQUALS("", errout.str());
2249+
22362250
check("void f() {\n" // #4032
22372251
" const std::string greeting(\"Hello World !!!\");\n"
22382252
" const std::string::size_type npos = greeting.rfind(\" \");\n"

0 commit comments

Comments
 (0)