From 4342fd254ca2d77e76b700e6b00dccf3e4fd2f61 Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Fri, 4 Nov 2011 19:21:19 +0100 Subject: [PATCH] Fixed #3266 (False positive on dangerous usage of .c_str()) --- lib/checkstl.cpp | 46 +++++++++++++++++++++++++++++----------------- lib/checkstl.h | 2 +- test/teststl.cpp | 11 ++++++----- 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 1780efa73..55880485c 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -1019,17 +1019,36 @@ void CheckStl::string_c_str() tok->next()->varId() > 0 && localvar.find(tok->next()->varId()) != localvar.end()) { string_c_strError(tok); - } else if (Token::Match(tok, "return %var% . c_str ( ) ;") && + } else if (Token::Match(tok, "[;{}] %var% = %var% . str ( ) . c_str ( ) ;") && tok->next()->varId() > 0 && - localvar.find(tok->next()->varId()) != localvar.end()) { + pointers.find(tok->next()->varId()) != pointers.end()) { string_c_strError(tok); + } else if (Token::Match(tok, "[;{}] %var% = %var% (") && + Token::simpleMatch(tok->tokAt(4)->link(), ") . c_str ( ) ;") && + tok->next()->varId() > 0 && + pointers.find(tok->next()->varId()) != pointers.end() && + Token::findmatch(_tokenizer->tokens(), ("std :: string " + tok->strAt(3) + " (").c_str())) { + string_c_strError(tok); + } + + // This part is inconclusive as the return type of the function + // might convert it to another string class implicitly. + // TODO: As soon as the symbol database stores the return value + // of a function, we can check if it's const char* and output a real error. + if (!_settings->inconclusive) + continue; + + if (Token::Match(tok, "return %var% . c_str ( ) ;") && + tok->next()->varId() > 0 && + localvar.find(tok->next()->varId()) != localvar.end()) { + string_c_strError(tok, true); } else if (Token::Match(tok, "return %var% . str ( ) . c_str ( ) ;") && tok->next()->varId() > 0 && localvar.find(tok->next()->varId()) != localvar.end()) { - string_c_strError(tok); + string_c_strError(tok, true); } else if (Token::simpleMatch(tok, "return std :: string (") && Token::simpleMatch(tok->tokAt(4)->link(), ") . c_str ( ) ;")) { - string_c_strError(tok); + string_c_strError(tok, true); } else if (Token::simpleMatch(tok, "return (") && Token::simpleMatch(tok->next()->link(), ") . c_str ( ) ;")) { // Check for "+ localvar" or "+ std::string(" inside the bracket @@ -1047,26 +1066,19 @@ void CheckStl::string_c_str() } if (is_implicit_std_string) - string_c_strError(tok); - } else if (Token::Match(tok, "[;{}] %var% = %var% . str ( ) . c_str ( ) ;") && - tok->next()->varId() > 0 && - pointers.find(tok->next()->varId()) != pointers.end()) { - string_c_strError(tok); - } else if (Token::Match(tok, "[;{}] %var% = %var% (") && - Token::simpleMatch(tok->tokAt(4)->link(), ") . c_str ( ) ;") && - tok->next()->varId() > 0 && - pointers.find(tok->next()->varId()) != pointers.end() && - Token::findmatch(_tokenizer->tokens(), ("std :: string " + tok->strAt(3) + " (").c_str())) { - string_c_strError(tok); + string_c_strError(tok, true); } } } } } -void CheckStl::string_c_strError(const Token *tok) +void CheckStl::string_c_strError(const Token* tok, bool is_inconlusive) { - reportError(tok, Severity::error, "stlcstr", "Dangerous usage of c_str()"); + if (is_inconlusive) + reportInconclusiveError(tok, Severity::error, "stlcstr", "Possible dangerous usage of c_str()"); + else + reportError(tok, Severity::error, "stlcstr", "Dangerous usage of c_str()"); } diff --git a/lib/checkstl.h b/lib/checkstl.h index 64c480e51..9d479452f 100644 --- a/lib/checkstl.h +++ b/lib/checkstl.h @@ -133,7 +133,7 @@ public: /** Check for common mistakes when using the function string::c_str() */ void string_c_str(); - void string_c_strError(const Token *tok); + void string_c_strError(const Token *tok, bool is_inconlusive=false); /** @brief %Check for use and copy auto pointer */ void checkAutoPointer(); diff --git a/test/teststl.cpp b/test/teststl.cpp index ec30ff56b..8edd73225 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -122,6 +122,7 @@ private: Settings settings; settings.addEnabled("style"); settings.addEnabled("performance"); + settings.inconclusive = true; // Tokenize.. Tokenizer tokenizer(&settings, this); @@ -1315,31 +1316,31 @@ private: " std::string errmsg;\n" " return errmsg.c_str();\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous usage of c_str()\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible dangerous usage of c_str()\n", errout.str()); check("const char *get_msg() {\n" " std::ostringstream errmsg;\n" " return errmsg.str().c_str();\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous usage of c_str()\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible dangerous usage of c_str()\n", errout.str()); check("const char *get_msg() {\n" " std::string errmsg;\n" " return std::string(\"ERROR: \" + errmsg).c_str();\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous usage of c_str()\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible dangerous usage of c_str()\n", errout.str()); check("const char *get_msg() {\n" " std::string errmsg;\n" " return (\"ERROR: \" + errmsg).c_str();\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous usage of c_str()\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible dangerous usage of c_str()\n", errout.str()); check("const char *get_msg() {\n" " std::string errmsg;\n" " return (\"ERROR: \" + std::string(\"crash me\")).c_str();\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous usage of c_str()\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3]: (error) Possible dangerous usage of c_str()\n", errout.str()); check("void f() {\n" " std::ostringstream errmsg;\n"