From ab8afec3ebc9cf2927e68a7a66131779f475de9d Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 16 Aug 2015 14:22:46 +0200 Subject: [PATCH] Refactorizations: - Avoid unnecessary loop iterations - Avoid unnecessary condition checking - Reduced code duplication in symboldatabase.cpp --- lib/checkbufferoverrun.cpp | 12 +++++------ lib/checkclass.cpp | 2 +- lib/checkmemoryleak.cpp | 18 ++++++++--------- lib/checknullpointer.cpp | 2 +- lib/checkother.cpp | 7 ++++--- lib/checkuninitvar.cpp | 1 + lib/symboldatabase.cpp | 41 ++++++++++++++------------------------ lib/tokenize.cpp | 9 +++++---- lib/tokenlist.cpp | 4 ++-- test/testmemleak.cpp | 10 +++++----- 10 files changed, 48 insertions(+), 58 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 6c060a434..219777411 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -471,10 +471,10 @@ void CheckBufferOverrun::checkFunctionParameter(const Token &ftok, unsigned int MathLib::bigint argsize = _tokenizer->sizeOfType(argument->typeStartToken()); if (argsize == 100) // unknown size argsize = 0; - while (Token::Match(tok2, "[ %num% ] [,)[]")) { + do { argsize *= MathLib::toLongNumber(tok2->strAt(1)); tok2 = tok2->tokAt(3); - } + } while (Token::Match(tok2, "[ %num% ] [,)[]")); MathLib::bigint arraysize = arrayInfo.element_size(); if (arraysize == 100) // unknown size @@ -548,7 +548,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector 0 && Token::Match(tok, "%varid% [", declarationId)) || (declarationId == 0 && Token::Match(tok, (varnames + " [").c_str()))) { - const Token *tok2 = tok; + const Token *tok2 = tok->next(); while (tok2->str() != "[") tok2 = tok2->next(); valueFlowCheckArrayIndex(tok2, arrayInfo); @@ -974,7 +974,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo std::size_t charactersAppend = 0; const Token *tok2 = tok; - while (tok2 && Token::Match(tok2, "strcat ( %varid% , %str% ) ;", declarationId)) { + while (Token::Match(tok2, "strcat ( %varid% , %str% ) ;", declarationId)) { charactersAppend += Token::getStrLength(tok2->tokAt(4)); if (charactersAppend >= (unsigned int)total_size) { bufferOverrunError(tok2, arrayInfo.varname()); @@ -1085,7 +1085,7 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable() const Variable * const var = symbolDatabase->getVariableFromVarId(i); if (var && var->isArray() && var->dimension(0) > 0) { const Token *tok = var->nameToken(); - while (tok && tok->str() != ";") { + do { if (tok->str() == "{") { if (Token::simpleMatch(tok->previous(), "= {")) tok = tok->link(); @@ -1093,7 +1093,7 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable() break; } tok = tok->next(); - } + } while (tok && tok->str() != ";"); if (!tok) break; if (tok->str() == "{") diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index b1cfec1a3..76fd29a3b 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -1670,7 +1670,7 @@ void CheckClass::checkConst() if (func->retDef->str() != "const") continue; } else if (Token::Match(previous->previous(), "*|& >")) { - const Token *temp = previous; + const Token *temp = previous->previous(); bool foundConst = false; while (!Token::Match(temp->previous(), ";|}|{|public:|protected:|private:")) { diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 0a32c754b..f302c03ac 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -2466,18 +2466,16 @@ void CheckMemoryLeakInClass::checkPublicFunctions(const Scope *scope, const Toke for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { if ((func->type == Function::eFunction || func->type == Function::eOperatorEqual) && func->access == Public && func->hasBody()) { - const Token *tok2 = func->token; - while (tok2->str() != "{") - tok2 = tok2->next(); - if (Token::Match(tok2, "{|}|; %varid% =", varid)) { - const CheckMemoryLeak::AllocType alloc = getAllocationType(tok2->tokAt(3), varid); + const Token *tok2 = func->functionScope->classStart->next(); + if (Token::Match(tok2, "%varid% =", varid)) { + const CheckMemoryLeak::AllocType alloc = getAllocationType(tok2->tokAt(2), varid); if (alloc != CheckMemoryLeak::No) - publicAllocationError(tok2, tok2->strAt(1)); - } else if (Token::Match(tok2, "{|}|; %type% :: %varid% =", varid) && - tok2->next()->str() == scope->className) { - const CheckMemoryLeak::AllocType alloc = getAllocationType(tok2->tokAt(5), varid); + publicAllocationError(tok2, tok2->str()); + } else if (Token::Match(tok2, "%type% :: %varid% =", varid) && + tok2->str() == scope->className) { + const CheckMemoryLeak::AllocType alloc = getAllocationType(tok2->tokAt(4), varid); if (alloc != CheckMemoryLeak::No) - publicAllocationError(tok2, tok2->strAt(3)); + publicAllocationError(tok2, tok2->strAt(2)); } } } diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 0428e2fad..b95e04782 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -61,7 +61,7 @@ void CheckNullPointer::parseFunctionCall(const Token &tok, std::listtokAt(2); - while (argtok && argtok->str() != ")") { + do { if (Token::Match(argtok,"%num% [,)]")) { if (MathLib::isInt(argtok->str()) && !_settings->library.isargvalid(functionToken, argnr, MathLib::toLongNumber(argtok->str()))) @@ -958,7 +958,7 @@ void CheckOther::invalidFunctionUsage() } argnr++; argtok = argtok->nextArgument(); - } + } while (argtok && argtok->str() != ")"); } } } @@ -1383,8 +1383,9 @@ void CheckOther::checkCommaSeparatedReturn() for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (tok->str() == "return") { + tok = tok->next(); while (tok && tok->str() != ";") { - if (Token::Match(tok, "[([{<]") && tok->link()) + if (tok->link() && Token::Match(tok, "[([{<]")) tok = tok->link(); if (!tok->isExpandedMacro() && tok->str() == "," && tok->linenr() != tok->next()->linenr()) diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index 7779a849c..586e3c93b 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -497,6 +497,7 @@ bool CheckUninitVar::checkScopeForVariable(const Token *tok, const Variable& var if (noreturn) *noreturn = true; + tok = tok->next(); while (tok && tok->str() != ";") { // variable is seen.. if (tok->varId() == var.declarationId()) { diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index bdab26afb..6f6294c3c 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1246,10 +1246,8 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti Token *tok2 = tok->next(); // Locate "]" - if (tok->next()->str() == "[") { - while (tok2 && tok2->str() == "[") - tok2 = tok2->link()->next(); - } + while (tok2 && tok2->str() == "[") + tok2 = tok2->link()->next(); Token *membertok = nullptr; if (Token::Match(tok2, ". %name%")) @@ -1571,37 +1569,27 @@ bool Function::argsMatch(const Scope *scope, const Token *first, const Token *se } // definition missing variable name - else if (first->next()->str() == "," && second->next()->str() != ",") { + else if ((first->next()->str() == "," && second->next()->str() != ",") || + (first->next()->str() == ")" && second->next()->str() != ")")) { second = second->next(); // skip default value assignment if (second->next()->str() == "=") { - while (!Token::Match(second->next(), ",|)")) - second = second->next(); - } - } else if (first->next()->str() == ")" && second->next()->str() != ")") { - second = second->next(); - // skip default value assignment - if (second->next()->str() == "=") { - while (!Token::Match(second->next(), ",|)")) + do { second = second->next(); + } while (!Token::Match(second->next(), ",|)")); } } else if (first->next()->str() == "[" && second->next()->str() != "[") second = second->next(); // function missing variable name - else if (second->next()->str() == "," && first->next()->str() != ",") { + else if ((second->next()->str() == "," && first->next()->str() != ",") || + (second->next()->str() == ")" && first->next()->str() != ")")) { first = first->next(); // skip default value assignment if (first->next()->str() == "=") { - while (!Token::Match(first->next(), ",|)")) - first = first->next(); - } - } else if (second->next()->str() == ")" && first->next()->str() != ")") { - first = first->next(); - // skip default value assignment - if (first->next()->str() == "=") { - while (!Token::Match(first->next(), ",|)")) + do { first = first->next(); + } while (!Token::Match(first->next(), ",|)")); } } else if (second->next()->str() == "[" && first->next()->str() != "[") first = first->next(); @@ -1632,8 +1620,9 @@ bool Function::argsMatch(const Scope *scope, const Token *first, const Token *se short_path.resize(short_path.size() - 4); // remove last name - while (!short_path.empty() && short_path[short_path.size() - 1] != ' ') - short_path.resize(short_path.size() - 1); + std::string::size_type lastSpace = short_path.find_last_of(' '); + if (lastSpace != std::string::npos) + short_path.resize(lastSpace+1); param = short_path + first->next()->str(); if (Token::Match(second->next(), param.c_str())) { @@ -2611,11 +2600,11 @@ void Function::addArguments(const SymbolDatabase *symbolDatabase, const Scope *s // skip default values if (tok->str() == "=") { - while (tok->str() != "," && tok->str() != ")") { + do { if (tok->link() && Token::Match(tok, "[{[(<]")) tok = tok->link(); tok = tok->next(); - } + } while (tok->str() != "," && tok->str() != ")"); } argumentList.push_back(Variable(nameTok, startTok, endTok, count++, Argument, argType, functionScope, &symbolDatabase->_settings->library)); diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index b6286988a..d67560458 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -2502,7 +2502,7 @@ static bool setVarIdParseDeclaration(const Token **tok, const std::map= 2 && executableScope && tok2 && tok2->str() == "[") { - const Token *tok3 = tok2; + const Token *tok3 = tok2->link()->next(); while (tok3 && tok3->str() == "[") { tok3 = tok3->link()->next(); } @@ -4059,6 +4059,7 @@ void Tokenizer::removeRedundantAssignment() // skip local class or struct if (Token::Match(tok2, "class|struct %type% {|:")) { // skip to '{' + tok2 = tok2->tokAt(2); while (tok2 && tok2->str() != "{") tok2 = tok2->next(); @@ -4915,10 +4916,10 @@ void Tokenizer::simplifyUndefinedSizeArray() tok = tok2->previous(); Token *end = tok2->next(); unsigned int count = 0; - while (Token::Match(end, "[ ] [;=[]")) { + do { end = end->tokAt(2); ++count; - } + } while (Token::Match(end, "[ ] [;=[]")); if (Token::Match(end, "[;=]")) { do { tok2->deleteNext(2); @@ -6125,7 +6126,7 @@ void Tokenizer::simplifyInitVar() if (!tok2->link() || (tok2->link()->strAt(1) == ";" && !Token::simpleMatch(tok2->linkAt(2), ") ("))) tok = initVar(tok); } else if (Token::Match(tok, "class|struct|union| %type% *| %name% ( &| %any% ) ,")) { - Token *tok1 = tok; + Token *tok1 = tok->tokAt(5); while (tok1->str() != ",") tok1 = tok1->next(); tok1->str(";"); diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp index 9e935e7f1..0c66c5e16 100644 --- a/lib/tokenlist.cpp +++ b/lib/tokenlist.cpp @@ -654,7 +654,7 @@ static void compilePrecedence3(Token *&tok, AST_state& state) if ((Token::Match(tok, "[+-!~*&]") || tok->tokType() == Token::eIncDecOp) && isPrefixUnary(tok, state.cpp)) { if (Token::Match(tok, "* [*,)]")) { - Token* tok2 = tok; + Token* tok2 = tok->next(); while (tok2->next() && tok2->str() == "*") tok2 = tok2->next(); if (Token::Match(tok2, "[>),]")) { @@ -736,7 +736,7 @@ static void compileMulDiv(Token *&tok, AST_state& state) while (tok) { if (Token::Match(tok, "[/%]") || (tok->str() == "*" && !tok->astOperand1())) { if (Token::Match(tok, "* [*,)]")) { - Token* tok2 = tok; + Token* tok2 = tok->next(); while (tok2->next() && tok2->str() == "*") tok2 = tok2->next(); if (Token::Match(tok2, "[>),]")) { diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 375c12c56..451e266db 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -5010,7 +5010,7 @@ private: " A::pd = new char[12];\n" " delete [] A::pd;\n" "}"); - ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:10]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); check("class A {\n" "private:\n" @@ -5022,7 +5022,7 @@ private: " delete [] pd;\n" " }\n" "};"); - ASSERT_EQUALS("[test.cpp:6]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:7]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); check("class A {\n" "private:\n" @@ -5036,7 +5036,7 @@ private: " pd = new char[12];\n" " delete [] pd;\n" "}"); - ASSERT_EQUALS("[test.cpp:9]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:10]: (warning) Possible leak in public function. The pointer 'pd' is not deallocated before it is allocated.\n", errout.str()); } void class18() { @@ -5457,7 +5457,7 @@ private: " delete data_;\n" " data_ = 0;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:16]: (warning) Possible leak in public function. The pointer 'data_' is not deallocated before it is allocated.\n" + ASSERT_EQUALS("[test.cpp:17]: (warning) Possible leak in public function. The pointer 'data_' is not deallocated before it is allocated.\n" "[test.cpp:18]: (error) Mismatching allocation and deallocation: Foo::data_\n", errout.str()); check("namespace NS\n" @@ -5480,7 +5480,7 @@ private: " delete data_;\n" " data_ = 0;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:16]: (warning) Possible leak in public function. The pointer 'data_' is not deallocated before it is allocated.\n" + ASSERT_EQUALS("[test.cpp:17]: (warning) Possible leak in public function. The pointer 'data_' is not deallocated before it is allocated.\n" "[test.cpp:18]: (error) Mismatching allocation and deallocation: Foo::data_\n", errout.str()); }