Moved remaining part of c_str() checking to checkstl.cpp. Fixed false positive #4157.

This commit is contained in:
PKEuS 2012-09-10 15:20:38 +02:00
parent 4e59e55229
commit 1e5d082251
5 changed files with 40 additions and 76 deletions

View File

@ -229,7 +229,7 @@ void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok,
// return temporary?
bool CheckAutoVariables::returnTemporary(const Token *tok) const
{
if (!Token::Match(tok, "return %var% ("))
if (!Token::Match(tok, "return %var% (") || !Token::simpleMatch(tok->linkAt(2), ") ;"))
return false;
const std::string &funcname(tok->next()->str());
@ -352,41 +352,3 @@ void CheckAutoVariables::errorInvalidDeallocation(const Token *tok)
"The deallocation of an auto-variable results in undefined behaviour. You should only free memory "
"that has been allocated dynamically.");
}
//---------------------------------------------------------------------------
// Return c_str
void CheckAutoVariables::returncstr()
{
// TODO: Move this to CheckStl::string_c_str
// locate function that returns a const char *..
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
// only check functions
if (scope->type != Scope::eFunction || !scope->function)
continue;
const Token *tok = scope->function->tokenDef;
// have we reached a function that returns a const char *
if (Token::simpleMatch(tok->tokAt(-3), "const char *")) {
for (const Token *tok2 = scope->classStart; tok2 && tok2 != scope->classEnd; tok2 = tok2->next()) {
// return pointer to temporary..
if (returnTemporary(tok2)) {
// report error..
errorReturnTempPointer(tok2);
}
}
}
}
}
void CheckAutoVariables::errorReturnTempPointer(const Token *tok)
{
reportError(tok, Severity::error, "returnTempPointer", "Pointer to temporary returned.");
}

View File

@ -51,7 +51,6 @@ public:
CheckAutoVariables checkAutoVariables(tokenizer, settings, errorLogger);
checkAutoVariables.autoVariables();
checkAutoVariables.returnPointerToLocalArray();
checkAutoVariables.returncstr();
}
/** Check auto variables */
@ -63,9 +62,6 @@ public:
/** Returning reference to local/temporary variable */
void returnReference();
/** Returning c_str to local variable */
void returncstr();
private:
bool isRefPtrArg(unsigned int varId);
bool isPtrArg(unsigned int varId);
@ -84,7 +80,6 @@ private:
void errorAutoVariableAssignment(const Token *tok, bool inconclusive);
void errorReturnReference(const Token *tok);
void errorReturnTempReference(const Token *tok);
void errorReturnTempPointer(const Token *tok);
void errorInvalidDeallocation(const Token *tok);
void errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname);
@ -95,7 +90,6 @@ private:
c.errorReturnPointerToLocalArray(0);
c.errorReturnReference(0);
c.errorReturnTempReference(0);
c.errorReturnTempPointer(0);
c.errorInvalidDeallocation(0);
c.errorReturnAddressOfFunctionParameter(0, "parameter");
}

View File

@ -1158,6 +1158,11 @@ void CheckStl::string_c_str()
} else if (Token::simpleMatch(tok, "return std :: string (") &&
Token::simpleMatch(tok->linkAt(4), ") . c_str ( ) ;")) {
string_c_strError(tok);
} else if (Token::Match(tok, "return %var% (") && Token::simpleMatch(tok->linkAt(2), ") . c_str ( ) ;")) {
const Token* fTok = _tokenizer->getFunctionTokenByName(tok->strAt(1).c_str());
const Function* func = symbolDatabase->findFunctionByToken(fTok);
if (func && Token::simpleMatch(func->tokenDef->tokAt(-3), "std :: string"))
string_c_strError(tok);
} else if (Token::simpleMatch(tok, "return (") &&
Token::simpleMatch(tok->next()->link(), ") . c_str ( ) ;")) {
// Check for "+ localvar" or "+ std::string(" inside the bracket

View File

@ -54,7 +54,6 @@ private:
// Check auto variables
checkAutoVariables.autoVariables();
checkAutoVariables.returnPointerToLocalArray();
checkAutoVariables.returncstr();
}
void run() {
@ -90,9 +89,6 @@ private:
TEST_CASE(returnReference6);
TEST_CASE(returnReference7);
// return c_str()..
TEST_CASE(returncstr);
// global namespace
TEST_CASE(testglobalnamespace);
@ -455,6 +451,15 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Reference to temporary returned.\n", errout.str());
check("std::string hello() {\n"
" return \"foo\";\n"
"}\n"
"\n"
"std::string &f() {\n"
" return hello().substr(1);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("class Foo;\n"
"Foo hello() {\n"
" return Foo();\n"
@ -621,33 +626,6 @@ private:
ASSERT_EQUALS("", errout.str());
}
void returncstr() {
check("std::string hello()\n"
"{\n"
" return \"hello\";\n"
"}\n"
"\n"
"const char *f()\n"
"{\n"
" return hello().c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (error) Pointer to temporary returned.\n", errout.str());
check("class Fred {\n"
" std::string hello();\n"
" const char *f();\n"
"};\n"
"std::string Fred::hello()\n"
"{\n"
" return \"hello\";\n"
"}\n"
"const char *Fred::f()\n"
"{\n"
" return hello().c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:11]: (error) Pointer to temporary returned.\n", errout.str());
}
void testglobalnamespace() {
check("class SharedPtrHolder\n"

View File

@ -1749,6 +1749,31 @@ private:
" return atoi(str.c_str());\n"
"}");
ASSERT_EQUALS("", errout.str());
check("std::string hello()\n"
"{\n"
" return \"hello\";\n"
"}\n"
"\n"
"const char *f()\n"
"{\n"
" return hello().c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:8]: (error) Dangerous usage of c_str(). The returned value by c_str() is invalid after this call.\n", errout.str());
check("class Fred {\n"
" std::string hello();\n"
" const char *f();\n"
"};\n"
"std::string Fred::hello()\n"
"{\n"
" return \"hello\";\n"
"}\n"
"const char *Fred::f()\n"
"{\n"
" return hello().c_str();\n"
"}\n");
ASSERT_EQUALS("[test.cpp:11]: (error) Dangerous usage of c_str(). The returned value by c_str() is invalid after this call.\n", errout.str());
}
void autoPointer() {