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

Implemented Function::nestedIn to be able to identify the scope the function belongs to, even if Function::functionScope.functionOf is not available. 

Refactorized usage of SymbolDatabase in checkOther:
- Don't copy Function instances in checkExpressionRange
- Simplifications by more accurate usage of information in database
This commit is contained in:
PKEuS 2012-05-24 08:40:43 -07:00
parent 97c4af44ca
commit e2bab4b6a3
6 changed files with 85 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
lib/checkother.cpp
View File

@ -1665,27 +1665,23 @@ void CheckOther::checkConstantFunctionParameter()
const SymbolDatabase * const symbolDatabase = _tokenizer->getSymbolDatabase();
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
if (i->type == Scope::eFunction && i->function && i->function->arg) {
for (const Token* tok = i->function->arg->next(); tok; tok = tok->nextArgument()) {
// TODO: False negatives. This pattern only checks for string.
// Investigate if there are other classes in the std
// namespace and add them to the pattern. There are
// streams for example (however it seems strange with
// const stream parameter).
if (Token::Match(tok, "const std :: string %var% [,)]")) {
passedByValueError(tok, tok->strAt(4));
} else if (Token::Match(tok, "const std :: %type% < std| ::| %type% > %var% [,)]")) {
passedByValueError(tok, Token::findsimplematch(tok->tokAt(5), ">")->strAt(1));
} else if (Token::Match(tok, "const std :: %type% < std| ::| %type% , std| ::| %type% > %var% [,)]")) {
passedByValueError(tok, Token::findsimplematch(tok->tokAt(7), ">")->strAt(1));
} else if (Token::Match(tok, "const %type% %var% [,)]")) {
// Check if type is a struct or class.
if (symbolDatabase->isClassOrStruct(tok->strAt(1))) {
passedByValueError(tok, tok->strAt(2));
}
}
}
for (unsigned int i = 1; i < symbolDatabase->getVariableListSize(); i++) {
const Variable* var = symbolDatabase->getVariableFromVarId(i);
if (!var || !var->isArgument() || !var->isClass() || !var->isConst() || var->isPointer() || var->isArray() || var->isReference())
continue;
const Token* tok = var->typeStartToken()->next();
// TODO: False negatives. This pattern only checks for string.
// Investigate if there are other classes in the std
// namespace and add them to the pattern. There are
// streams for example (however it seems strange with
// const stream parameter).
if (Token::Match(tok, "std :: string|wstring")) {
passedByValueError(tok, var->name());
} else if (Token::Match(tok, "std :: %type% <")) {
passedByValueError(tok, var->name());
} else if (var->type() || symbolDatabase->isClassOrStruct(tok->str())) { // Check if type is a struct or class.
passedByValueError(tok, var->name());
}
}
}
@ -1851,7 +1847,7 @@ void CheckOther::strPlusChar()
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "[=(] %str% + %any%")) {
// char constant..
if (tok->strAt(3)[0] == '\'')
if (tok->tokAt(3)->type() == Token::eChar)
strPlusCharError(tok->next());
// char variable..
@ -1960,6 +1956,7 @@ void CheckOther::mathfunctionCallError(const Token *tok, const unsigned int numP
} else
reportError(tok, Severity::error, "wrongmathcall", "Passing value " " to " "() leads to undefined result");
}
//---------------------------------------------------------------------------
//---------------------------------------------------------------------------
void CheckOther::checkCCTypeFunctions()
@ -1976,6 +1973,7 @@ void CheckOther::cctypefunctionCallError(const Token *tok, const std::string &fu
{
reportError(tok, Severity::error, "wrongcctypecall", "Passing value " + value + " to " + functionName + "() cause undefined behavior, which may lead to a crash");
}
//---------------------------------------------------------------------------
//---------------------------------------------------------------------------
/** Is there a function with given name? */
@ -2178,29 +2176,20 @@ void CheckOther::checkDuplicateBranch()
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
const Token* tok = scope->classDef;
if ((scope->type != Scope::eIf && scope->type != Scope::eElseIf) || !tok)
if (scope->type != Scope::eIf && scope->type != Scope::eElseIf)
continue;
if (tok->str() == "else")
tok = tok->next();
// check all the code in the function for if (..) else
if (tok && tok->next() && Token::simpleMatch(tok->next()->link(), ") {") &&
Token::simpleMatch(tok->next()->link()->next()->link(), "} else {")) {
if (Token::simpleMatch(scope->classEnd, "} else {")) {
// save if branch code
std::string branch1 = tok->next()->link()->tokAt(2)->stringifyList(tok->next()->link()->next()->link());
// find else branch
const Token *tok1 = tok->next()->link()->next()->link();
std::string branch1 = scope->classStart->next()->stringifyList(scope->classEnd);
// save else branch code
std::string branch2 = tok1->tokAt(3)->stringifyList(tok1->linkAt(2));
std::string branch2 = scope->classEnd->tokAt(3)->stringifyList(scope->classEnd->linkAt(2));
// check for duplicates
if (branch1 == branch2)
duplicateBranchError(tok, tok1->tokAt(2));
duplicateBranchError(scope->classDef, scope->classEnd->tokAt(1));
}
}
}
@ -2329,11 +2318,11 @@ namespace {
struct FuncFilter {
FuncFilter(const Scope *scope, const Token *tok): _scope(scope), _tok(tok) {}
bool operator()(const Function &func) const {
bool matchingFunc = func.type == Function::eFunction &&
_tok->str() == func.token->str();
bool operator()(const Function* func) const {
bool matchingFunc = func->type == Function::eFunction &&
_tok->str() == func->token->str();
// either a class function, or a global function with the same name
return (_scope && _scope == func.functionScope && matchingFunc) ||
return (_scope && _scope == func->nestedIn && matchingFunc) ||
(!_scope && matchingFunc);
}
const Scope *_scope;
@ -2341,7 +2330,7 @@ namespace {
};
bool inconclusiveFunctionCall(const SymbolDatabase *symbolDatabase,
const std::list<Function> &constFunctions,
const std::list<const Function*> &constFunctions,
const ExpressionTokens &tokens)
{
const Token *start = tokens.start;
@ -2366,7 +2355,7 @@ namespace {
// hard coded list of safe, no-side-effect functions
if (v == 0 && Token::Match(prev, "strcmp|strncmp|strlen|memcmp|strcasecmp|strncasecmp"))
return false;
std::list<Function>::const_iterator it = std::find_if(constFunctions.begin(),
std::list<const Function*>::const_iterator it = std::find_if(constFunctions.begin(),
constFunctions.end(),
FuncFilter(v ? v->type(): 0, prev));
if (it == constFunctions.end())
@ -2380,7 +2369,7 @@ namespace {
class Expressions {
public:
Expressions(const SymbolDatabase *symbolDatabase, const
std::list<Function> &constFunctions)
std::list<const Function*> &constFunctions)
: _start(0),
_lastTokens(0),
_symbolDatabase(symbolDatabase),
@ -2424,39 +2413,29 @@ namespace {
const Token *_start;
ExpressionTokens *_lastTokens;
const SymbolDatabase *_symbolDatabase;
const std::list<Function> &_constFunctions;
const std::list<const Function*> &_constFunctions;
};
bool notconst(const Function &func)
bool notconst(const Function* func)
{
return !func.isConst;
return !func->isConst;
}
void getConstFunctions(const SymbolDatabase *symbolDatabase, std::list<Function> &constFunctions)
void getConstFunctions(const SymbolDatabase *symbolDatabase, std::list<const Function*> &constFunctions)
{
std::list<Scope>::const_iterator scope;
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
std::list<Function>::const_iterator func;
// only add const functions that do not have a non-const overloaded version
// since it is pretty much impossible to tell which is being called.
typedef std::map<std::string, std::list<Function> > StringFunctionMap;
typedef std::map<std::string, std::list<const Function*> > StringFunctionMap;
StringFunctionMap functionsByName;
for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) {
StringFunctionMap::iterator it = functionsByName.find(func->tokenDef->str());
Scope *currScope = const_cast<Scope*>(&*scope);
if (it == functionsByName.end()) {
std::list<Function> tmp;
tmp.push_back(*func);
tmp.back().functionScope = currScope;
functionsByName[func->tokenDef->str()] = tmp;
} else {
it->second.push_back(*func);
it->second.back().functionScope = currScope;
}
functionsByName[func->tokenDef->str()].push_back(&*func);
}
for (StringFunctionMap::iterator it = functionsByName.begin();
it != functionsByName.end(); ++it) {
std::list<Function>::const_iterator nc = std::find_if(it->second.begin(), it->second.end(), notconst);
std::list<const Function*>::const_iterator nc = std::find_if(it->second.begin(), it->second.end(), notconst);
if (nc == it->second.end()) {
// ok to add all of them
constFunctions.splice(constFunctions.end(), it->second);
@ -2467,7 +2446,7 @@ namespace {
}
void CheckOther::checkExpressionRange(const std::list<Function> &constFunctions,
void CheckOther::checkExpressionRange(const std::list<const Function*> &constFunctions,
const Token *start,
const Token *end,
const std::string &toCheck)
@ -2536,7 +2515,7 @@ void CheckOther::checkExpressionRange(const std::list<Function> &constFunctions,
}
}
void CheckOther::complexDuplicateExpressionCheck(const std::list<Function> &constFunctions,
void CheckOther::complexDuplicateExpressionCheck(const std::list<const Function*> &constFunctions,
const Token *classStart,
const std::string &toCheck,
const std::string &alt)
@ -2598,7 +2577,7 @@ void CheckOther::checkDuplicateExpression()
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
std::list<Scope>::const_iterator scope;
std::list<Function> constFunctions;
std::list<const Function*> constFunctions;
getConstFunctions(symbolDatabase, constFunctions);
for (scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {

4
lib/checkother.h
View File

@ -397,12 +397,12 @@ private:
"* Comparisons of modulo results that are always true/false.\n";
}
void checkExpressionRange(const std::list<Function> &constFunctions,
void checkExpressionRange(const std::list<const Function*> &constFunctions,
const Token *start,
const Token *end,
const std::string &toCheck);
void complexDuplicateExpressionCheck(const std::list<Function> &constFunctions,
void complexDuplicateExpressionCheck(const std::list<const Function*> &constFunctions,
const Token *classStart,
const std::string &toCheck,
const std::string &alt);

18
lib/symboldatabase.cpp
View File

@ -217,6 +217,9 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
// save the function name location
function.tokenDef = funcStart;
// save the function parent scope
function.nestedIn = scope;
// operator function
if (function.tokenDef->str().find("operator") == 0) {
function.isOperator = true;
@ -329,12 +332,11 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti
scope->functionList.push_back(function);
const Token *tok2 = funcStart;
Scope *functionOf = scope;
addNewFunction(&scope, &tok2);
if (scope) {
scope->functionOf = functionOf;
scope->function = &functionOf->functionList.back();
scope->functionOf = function.nestedIn;
scope->function = &function.nestedIn->functionList.back();
scope->function->functionScope = scope;
}
@ -963,6 +965,7 @@ Function* SymbolDatabase::addGlobalFunctionDecl(Scope*& scope, const Token *argS
function.isInline = false;
function.hasBody = false;
function.type = Function::eFunction;
function.nestedIn = scope;
scope->functionList.push_back(function);
return &scope->functionList.back();
@ -1391,6 +1394,15 @@ void SymbolDatabase::printOut(const char *title) const
std::cout << " token: " << _tokenizer->list.fileLine(func->token) << std::endl;
std::cout << " arg: " << _tokenizer->list.fileLine(func->arg) << std::endl;
}
std::cout << " nestedIn: ";
if (func->nestedIn) {
std::cout << func->nestedIn->className << " " << func->nestedIn->type;
if (func->nestedIn->classDef)
std::cout << " " << _tokenizer->list.fileLine(func->nestedIn->classDef) << std::endl;
else
std::cout << std::endl;
} else
std::cout << "Unknown" << std::endl;
std::cout << " functionScope: ";
if (func->functionScope) {
std::cout << func->functionScope->className << " "

2
lib/symboldatabase.h
View File

@ -367,6 +367,7 @@ public:
token(NULL),
arg(NULL),
functionScope(NULL),
nestedIn(NULL),
type(eFunction),
access(Public),
hasBody(false),
@ -395,6 +396,7 @@ public:
const Token *token; // function name token in implementation
const Token *arg; // function argument start '('
Scope *functionScope; // scope of function body
Scope* nestedIn; // Scope the function is declared in
std::list<Variable> argumentList; // argument list
Type type; // constructor, destructor, ...
AccessControl access; // public/protected/private

54
test/testother.cpp
View File

@ -2158,39 +2158,27 @@ private:
}
void trac1132() {
std::istringstream code("#include <iostream>\n"
"class Lock\n"
"{\n"
"public:\n"
" Lock(int i)\n"
" {\n"
" std::cout << \"Lock \" << i << std::endl;\n"
" }\n"
" ~Lock()\n"
" {\n"
" std::cout << \"~Lock\" << std::endl;\n"
" }\n"
"};\n"
"int main()\n"
"{\n"
" Lock(123);\n"
" std::cout << \"hello\" << std::endl;\n"
" return 0;\n"
"}\n"
);
errout.str("");
Settings settings;
Tokenizer tokenizer(&settings, this);
tokenizer.tokenize(code, "trac1132.cpp");
tokenizer.simplifyTokenList();
CheckOther checkOther(&tokenizer, &settings, this);
checkOther.checkMisusedScopedObject();
ASSERT_EQUALS("[trac1132.cpp:16]: (error) Instance of \"Lock\" object destroyed immediately.\n", errout.str());
check("#include <iostream>\n"
"class Lock\n"
"{\n"
"public:\n"
" Lock(int i)\n"
" {\n"
" std::cout << \"Lock \" << i << std::endl;\n"
" }\n"
" ~Lock()\n"
" {\n"
" std::cout << \"~Lock\" << std::endl;\n"
" }\n"
"};\n"
"int main()\n"
"{\n"
" Lock(123);\n"
" std::cout << \"hello\" << std::endl;\n"
" return 0;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:16]: (error) Instance of \"Lock\" object destroyed immediately.\n", errout.str());
}
void trac3693() {

6
test/testsymboldatabase.cpp
View File

@ -496,7 +496,7 @@ private:
ASSERT(function && function->token->str() == "func");
ASSERT(function && function->token == tokenizer.tokens()->next());
ASSERT(function && function->hasBody);
ASSERT(function && function->functionScope == scope && scope->function == function);
ASSERT(function && function->functionScope == scope && scope->function == function && function->nestedIn != scope);
}
}
@ -517,7 +517,7 @@ private:
ASSERT(function && function->token->str() == "func");
ASSERT(function && function->token == tokenizer.tokens()->tokAt(4));
ASSERT(function && function->hasBody && function->isInline);
ASSERT(function && function->functionScope == scope && scope->function == function);
ASSERT(function && function->functionScope == scope && scope->function == function && function->nestedIn == db->findScopeByName("Fred"));
}
}
@ -557,7 +557,7 @@ private:
ASSERT(function && function->token->str() == "func");
ASSERT(function && function->token == tokenizer.tokens()->tokAt(12));
ASSERT(function && function->hasBody && !function->isInline);
ASSERT(function && function->functionScope == scope && scope->function == function);
ASSERT(function && function->functionScope == scope && scope->function == function && function->nestedIn == db->findScopeByName("Fred"));
}
}