diff --git a/lib/checkother.cpp b/lib/checkother.cpp index f04bab32f..fa0b228f3 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -230,38 +230,7 @@ void CheckOther::clarifyConditionError(const Token *tok, bool assign, bool boolo } //--------------------------------------------------------------------------- -// Clarify (meaningless) statements like *foo++; with parantheses.teStatement() -{ - if (!_settings->isEnabled("style")) - return; - - fovoid CheckOther::clarifyStatementves. This pattern only checks for string. - // Investifor (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::Match(tok, "[{};] * %var%")) { - tok = tok->tokAt(3); - while (tok) { - if (tok->str() == "[") - tok = tok->link()->next(); - if (Token::Match(tok, ".|:: %var%")) { - if (tok->strAt(2) == "(") - tok = tok->linkAt(2)->next(); - else - tok = tok->tokAt(2); - } else - break; - } - if (Token::Match(tok, "++|-- [;,]")) - clarifyStatementError(tok); - } - } -} - -void CheckOther::clarifyStatementError(const Token *tok) -{ - reportError(tok, Severity::warning, "clarifyStatement", "Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n" - "A statement like '*A++;' might not do what you intended. 'operator*' is executed before postfix 'operator++'. " - "Thus, the dereference is meaningless. Did you intend to write '(*A)++;'?ossible unexpanded macro hiding for/while.. - else if (tok->str() != "else" &&if (bool & bool) -> if (bool && bool) +// if (bool & bool) -> if (bool && bool) // if (bool | bool) -> if (bool || bool) //--------------------------------------------------------------------------- void CheckOther::checkBitwiseOnBoolean() @@ -2195,36 +2164,50 @@ void CheckOther::incorrectStringBooleanError(const Token *tok, const std::string //----------------------------------------------------------------------------- // check for duplicate expressions in if statements // if (a) { } else if (a) { } -//----------------------------ok1->linkAt(3), ") {")) { - // get the expression from the token stream - expression = tok1->tokAt(4)->stringifyList(tok1->linkAt(3)); +//----------------------------------------------------------------------------- - // try to look up the expression to check for duplicates - std::map::iterator it = expressionMap.find(expression); +static bool expressionHasSideEffects(const Token *first, const Token *last) +{ + for (const Token *tok = first; tok != last->next(); tok = tok->next()) { + // check for assignment + if (tok->isAssignmentOp()) + return true; - // found a duplicate - if (it != expressionMap.end()) { - // check for expressions that have side effects and ignore them - if (!expressionHasSideEffects(tok1->tokAt(4), tok1->linkAt(3)->previous())) - duplicateIfError(it->second, tok1->next()); - } + // check for inc/dec + else if (tok->type() == Token::eIncDecOp) + return true; - // not a duplicate expression so save it and its location - else - expressionMap.insert(std::make_pair(expression, tok1->next())); - - // find the next else if (...) statement - tok1 = tok1->linkAt(3)->next()->link(); - } + // check for function call + else if (Token::Match(tok, "%var% (") && + !(Token::Match(tok, "c_str|string") || tok->isStandardType())) + return true; } + + return false; } -void CheckOther::duplicateIf else if (Token::simpleMatch(tok->next()->link(), ") ;")) { - for (const Token* tok2 = tok->tokAt(2); tok2 != tok->linkAt(1); tok2 = tok2->next()) { - If", "Found duplicate if expressions.\n" - "Finding the same expression more than once is suspicious and might indicate " - "a cut and paste or logic error. Please examine this code carefully to determine " - "if it is next else if (...) statement +void CheckOther::checkDuplicateIf() +{ + if (!_settings->isEnabled("style")) + return; + + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + + for (std::list::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) { + const Token* const tok = scope->classDef; + // only check if statements + if (scope->type != Scope::eIf || !tok) + continue; + + std::map expressionMap; + + // get the expression from the token stream + std::string expression = tok->tokAt(2)->stringifyList(tok->next()->link()); + + // save the expression and its location + expressionMap.insert(std::make_pair(expression, tok)); + + // find the next else if (...) statement const Token *tok1 = scope->classEnd; // check all the else if (...) statements @@ -3078,41 +3061,58 @@ void CheckOther::checkRedundantCopy() for (const Token *tok = _tokenizer->tokens(); tok; tok=tok->next()) { const char *expect_end_token; - if (Token::Match(tok, "const %type% %var% =")) - Token::simpleMatch(tok1->linkAt(3), ") {")) { - // get the -// Check for incompletely filled buffers. Token::simpleMatch(tok1->linkAt(3), ") {")) { - // get the -void CheckOther::checkIncompleteArrayFill() + if (Token::Match(tok, "const %type% %var% =")) { + //match "const A a =" usage + expect_end_token = ";"; + } else if (Token::Match(tok, "const %type% %var% (")) { + //match "const A a (" usage + expect_end_token = ")"; + } else { + continue; + } + + if (tok->strAt(1) == tok->strAt(4)) //avoid "const A a = A();" + continue; + if (!symbolDatabase->isClassOrStruct(tok->next()->str())) //avoid when %type% is standard type + continue; + const Token *var_tok = tok->tokAt(2); + tok = tok->tokAt(4); + while (tok &&Token::Match(tok,"%var% .")) + tok = tok->tokAt(2); + if (!Token::Match(tok, "%var% (")) + break; + const Token *match_end = (tok->next()->link()!=NULL)?tok->next()->link()->next():NULL; + if (match_end==NULL || !Token::Match(match_end,expect_end_token)) //avoid usage like "const A a = getA()+3" + break; + const Token *fToken = _tokenizer->getFunctionTokenByName(tok->str().c_str()); + if (fToken &&fToken->previous() && fToken->previous()->str() == "&") { + redundantCopyError(var_tok,var_tok->str()); + } + } +} +void CheckOther::redundantCopyError(const Token *tok,const std::string& varname) { - if (!_settings->inconclusive || ion = tok1->tokAt(4)->stringifyList(tok1->linkAt(3)); + reportError(tok, Severity::performance,"redundantCopyLocalConst", + "Use const reference for "+varname+" to avoid unnecessary data copying.\n" + "The const "+varname+" gets a copy of the data since const reference is not used. You can avoid the unnecessary data copying by converting "+varname+" to const reference instead of just const."); +} - // try to look up the expression to check for duplicates - for (const Token* tok = _tokenizer->list.front(); tok; tok = tok->next()) { - if (Token::Match(tok, "memset|memcpy|memmove ( %var% ,") && Token::Match(tok->linkAt(1)->tokAt(-2), ", %num% )")) { - const Variable* var = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId()); - if (!var || !var->isArray() || var->dimensions().empty() || !var->dimension(0)) - continue; +//--------------------------------------------------------------------------- +// Checking for shift by negative values +//--------------------------------------------------------------------------- - if (MathLib::toLongNumber(tok->linkAt(1)->strAt(-1)) == var->dimension(0)) { - unsigned int size = _tokenizer->sizeOfType(var->typeStartToken()); - if ((size != 1 && size != 100 && size != 0) || Token::Match(var->typeEndToken(), "*")) - incompleteArrayFillError(tok, var->name(), tok->str(), false); - else if (var->typeStartToken()->str() == "bool" && _settings->isEnabled("portability")) // sizeof(bool) is not 1 on all platforms - incompleteArrayFillError(tok, var->name(), tok->str(), true); - } +void CheckOther::checkNegativeBitwiseShift() +{ + for (const Token *tok = _tokenizer->tokens(); tok ; tok = tok->next()) { + if (Token::Match(tok,"%var% >>|<< %num%") || Token::Match(tok,"%num >>|<< %num%")) { + if ((tok->strAt(2))[0] == '-') + negativeBitwiseShiftError(tok); } } } -void CheckOther::incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean) + +void CheckOther::negativeBitwiseShiftError(const Token *tok) { - if (boolean) - reportError(tok, Severity::portability, "incompleteArrayFill", - "Array '" + buffer + "' might be filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n" - "The array '" + buffer + "' is filled incompletely. The function '" + function + "()' needs the size given in bytes, but the type 'bool' is larger than 1 on some platforms. Did you forget to multiply the size with 'sizeof(*" + buffer + ")'?", true); - else - reportError(tok, Severity::warning, "incompleteArrayFill", - "Array '" + buffer + "' is filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n" - "The array '" + buffer + "' is filled incompletely. The function '" + function + "()' needs the size given in bytes, but an element of the given array is larger than one byte. Did you forget to multiply the size with 'sizeof(*" + buffer + ")'?", true); + reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value."); } diff --git a/lib/checkother.h b/lib/checkother.h index ec32d71a4..98c3f5ba3 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -74,7 +74,6 @@ public: checkOther.clarifyCondition(); // not simplified because ifAssign checkOther.checkComparisonOfBoolExpressionWithInt(); checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574) - checkOther.checkIncompleteArrayFill(); } /** @brief Run checks against the simplified token list */ @@ -83,7 +82,6 @@ public: // Checks checkOther.clarifyCalculation(); - checkOther.clarifyStatement(); checkOther.checkConstantFunctionParameter(); checkOther.checkIncompleteStatement(); @@ -112,8 +110,7 @@ public: /** @brief Clarify calculation for ".. a * b ? .." */ void clarifyCalculation(); - /** @brief Suspicious condition (assignment+comparison) Suspicious statement like '*A++;' */ - void clarifyStatementignment+comparison) */ + /** @brief Suspicious condition (assignment+comparison) */ void clarifyCondition(); /** @brief Are there C-style pointer casts in a c++ file? */ @@ -241,15 +238,14 @@ public: void checkDoubleFree(); void doubleFreeError(const Token *tok, const std::string &varname); - /** @brief %Check for code creating redu /** @brief %Check for buffers that are filled incompletely with memset and similar functions */ - void checkIncompleteArrayFill redundant copies */ + /** @brief %Check for code creating redundant copies */ void checkRedundantCopy(); /** @brief %Check for bitwise operation with negative right operand */ void checkNegativeBitwiseShift(); private: - // Error messages.clarifyStatementError(const Token* tokor messages.. + // Error messages.. void clarifyCalculationError(const Token *tok, const std::string &op); void clarifyConditionError(const Token *tok, bool assign, bool boolop); void sizeofsizeofError(const Token *tok); @@ -301,7 +297,7 @@ private: void pointerPositiveError(const Token *tok, bool inconclusive); void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1); - void SuspiciousSemicolonError(con void incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool booleanconst Token *tok); + void SuspiciousSemicolonError(const Token *tok); void doubleCloseDirError(const Token *tok, const std::string &varname); void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal); void negativeBitwiseShiftError(const Token *tok); @@ -341,7 +337,6 @@ private: c.redundantAssignmentInSwitchError(0, "varname"); c.redundantOperationInSwitchError(0, "varname"); c.switchCaseFallThrough(0); -clarifyStatementError(0lThrough(0); c.selfAssignmentError(0, "varname"); c.assignmentInAssertError(0, "varname"); c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true); @@ -361,7 +356,6 @@ clarifyStatementError(0lThrough(0); c.duplicateBreakError(0, false); c.unreachableCodeError(0, false); c.unsignedLessThanZeroError(0, "varname", false); - c.incompleteArrayFillError(0, "buffer", "memset", falselse); c.unsignedPositiveError(0, "varname", false); c.pointerLessThanZeroError(0, false); c.pointerPositiveError(0, false); @@ -420,8 +414,7 @@ clarifyStatementError(0lThrough(0); "* comparison of a boolean expression with an integer other than 0 or 1\n" "* suspicious condition (assignment+comparison)\n" "* suspicious condition (runtime comparison of string literals)\n" - "* suspic - "* Array filled incompletely using memset/memcpy/memmovuspicious condition (string literals as boolean)\n" + "* suspicious condition (string literals as boolean)\n" "* duplicate break statement\n" "* unreachable code\n" "* testing if unsigned variable is negative\n" @@ -433,4 +426,16 @@ clarifyStatementError(0lThrough(0); } void checkExpressionRange(const std::list &constFunctions, - \ No newline at end of file + const Token *start, + const Token *end, + const std::string &toCheck); + + void complexDuplicateExpressionCheck(const std::list &constFunctions, + const Token *classStart, + const std::string &toCheck, + const std::string &alt); +}; +/// @} +//--------------------------------------------------------------------------- +#endif + diff --git a/test/testother.cpp b/test/testother.cpp index aefbb5703..8b797f9b1 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -116,7 +116,7 @@ private: TEST_CASE(memsetZeroBytes); TEST_CASE(sizeofForArrayParameter); - TEST_CASE(sizeofForNumericPar TEST_CASE(clarifyStatementParameter); + TEST_CASE(sizeofForNumericParameter); TEST_CASE(clarifyCalculation); @@ -159,8 +159,7 @@ private: TEST_CASE(checkForSuspiciousSemicolon1); TEST_CASE(checkForSuspiciousSemicolon2); - TEST_CASE(checkDoub - TEST_CASE(incompleteArrayFilloubleFree); + TEST_CASE(checkDoubleFree); TEST_CASE(checkRedundantCopy); @@ -169,7 +168,6 @@ private: void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) { // Clear the error buffer.. - errout.straddEnabled("portability.. errout.str(""); Settings settings; @@ -3030,21 +3028,15 @@ private: "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:5]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" - "[test.cpp:6]: (warning) Comparison of modu "}\n" - ); - ASSERT_EQUALS("", errout.str()); + "[test.cpp:6]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" + "[test.cpp:7]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n", errout.str()); - check("void f(i || x <= 3.\n", errout.str()); - - check("void f(int x) {\n" - x > 5 && x != l conjunction always evaluates to false: x < 1 && x > 1.\n", errout.str()); - - check("void "}\n" - ); - ASSERT_EQUALS("[test.cpp:2]: (warning)> 5) && (x != 1) conjunction always evaluates to false: x < 1 && x > 1.\n", errout.str()); - - check("void f(int x) {\n" -sult is predetermined, because it is always less than 5.\n" + check("void f(bool& b1, bool& b2) {\n" + " b1 = bar() % 5 < 889;\n" + " if(x[593] % 5 <= 5)\n" + " b2 = x.a % 5 == 5;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:3]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n" "[test.cpp:4]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n", errout.str()); } @@ -3640,48 +3632,7 @@ sult is predetermined, because it is always less than 5.\n" ); ASSERT_EQUALS("[test.cpp:2]: (warning) Comparison of a boolean value using relational (<, >, <= or >=) operator.\n", errout.str()); - check("voidvoid clarifyStatement() { - check("char* f(char* c) {\n" - " *c++;\n" - " return c;n - void clarifyCondition1() { - check("void f() {\n" - Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str()); - - check("char* f(char** c) {\n" - " *c[5]--;\n" - " return *c;n - void clarifyCondition1() { - check("void f() {\n" - Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str()); - - check("void f(Foo f) {\n" - " *f.a++;n - void clarifyCondition1() { - check("void f() {\n" - Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str()); - - check("void f(Foo f) {\n" - " *f.a[5].v[3]++;n - void clarifyCondition1() { - check("void f() {\n" - Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str()); - - check("void f(Foo f) {\n" - " *f.a(1, 5).v[x + y]++;n - void clarifyCondition1() { - check("void f() {\n" - Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str()); - - check("char* f(char* c) {\n" - " (*c)++;\n" - " return cl 0" - " bytes of \'p\'\n", errout.str()); - - - check("void f(char* c) {\n" - " bar(*c++) check("void f(int x) {\n" - " if (x < 1 && x > 1check("void f(bool x ) {\n" + check("void f(bool x ) {\n" " if ( false <= x )\n" " a++;\n" "}\n" @@ -5495,54 +5446,396 @@ sult is predetermined, because it is always less than 5.\n" ASSERT_EQUALS("", errout.str()); check( - - void incompleteArrayFill() { - check("void f() {\n" - " int a[5];\n" - " memset(a, 123, 5);\n" - " memcpy(a, b, 5);\n" - " memmove(a, b, 5);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n" - "[test.cpp:4]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memcpy()' with 'sizeof(*a)'?\n" - "[test.cpp:5]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memmove()' with 'sizeof(*a)'al (<, >, <= or >=) operator.\n", errout.str()); + "void f() {\n" + " char *p = malloc(100);\n" + " if (x) {\n" + " free(p);\n" + " exit();\n" + " }\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); - check("void Foo* a[5];\n" - " memset(a, 'a', 5);\n" - "}"); - ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", errout.str()); + check( + "void f() {\n" + " char *p = malloc(100);\n" + " if (x) {\n" + " free(p);\n" + " x = 0;\n" + " }\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("[test.cpp:7]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); - check("class Foo {int a; int b;};\n" - "void f() {\n" - " Foo a[5];\n" - " memset(a, 'a', 5);\n" + check( + "void f() {\n" + " char *p = do_something();\n" + " free(p);\n" + " p = do_something();\n" + " free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " g_free(p);\n" + " g_free(p);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p, char *r) {\n" + " g_free(p);\n" + " g_free(r);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " g_free(p);\n" + " getNext(&p);\n" + " g_free(p);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " g_free(p);\n" + " bar();\n" + " g_free(p);\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " delete p;\n" + " delete p;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p, char *r) {\n" + " delete p;\n" + " delete r;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete p;\n" + " getNext(&p);\n" + " delete p;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete p;\n" + " bar();\n" + " delete p;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p) {\n" + " delete[] p;\n" + " delete[] p;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "void foo(char *p, char *r) {\n" + " delete[] p;\n" + " delete[] r;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete[] p;\n" + " getNext(&p);\n" + " delete[] p;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(char *p) {\n" + " delete[] p;\n" + " bar();\n" + " delete[] p;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + + check( + "~LineMarker() {\n" + " delete pxpm;\n" + "}\n" + "LineMarker &operator=(const LineMarker &) {\n" + " delete pxpm;\n" + " pxpm = NULL;\n" + " return *this;\n" + "}" + ); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo()\n" + "{\n" + " int* ptr = NULL;\n" + " try\n" + " {\n" + " ptr = new int(4);\n" + " }\n" + " catch(...)\n" + " {\n" + " delete ptr;\n" + " throw;\n" + " }\n" + " delete ptr;\n" + "}" + ); + ASSERT_EQUALS("", errout.str()); + + check( + "int foo()\n" + "{\n" + " int* a = new int;\n" + " bool doDelete = true;\n" + " if (a != 0)\n" + " {\n" + " doDelete = false;\n" + " delete a;\n" + " }\n" + " if(doDelete)\n" + " delete a;\n" + " return 0;\n" + "}" + ); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x = NULL;\n" + " while(1) {\n" + " x = new char[100];\n" + " if (y++ > 100)\n" + " break;\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}" + ); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x = NULL;\n" + " for (int i = 0; i < 10000; i++) {\n" + " x = new char[100];\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}" + ); + ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x = NULL;\n" + " while (isRunning()) {\n" + " x = new char[100];\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}" + ); + ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x = NULL;\n" + " while (isRunning()) {\n" + " x = malloc(100);\n" + " free(x);\n" + " }\n" + " free(x);\n" + "}" + ); + ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'x' is freed twice.\n", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x = NULL;\n" + " for (;;) {\n" + " x = new char[100];\n" + " if (y++ > 100)\n" + " break;\n" + " delete[] x;\n" + " }\n" + " delete[] x;\n" + "}" + ); + ASSERT_EQUALS("", errout.str()); + + check( + "void foo(int y)\n" + "{\n" + " char * x = NULL;\n" + " do {\n" + " x = new char[100];\n" + " if (y++ > 100)\n" + " break;\n" + " delete[] x;\n" + " } while (1);\n" + " delete[] x;\n" + "}" + ); + ASSERT_EQUALS("", errout.str()); + + check( + "void f()\n" + "{\n" + " char *p = 0;\n" + " if (x < 100) {\n" + " p = malloc(10);\n" + " free(p);\n" + " }\n" + " free(p);\n" + "}" + ); + ASSERT_EQUALS("[test.cpp:8]: (error) Memory pointed to by 'p' is freed twice.\n", errout.str()); + } + + void check_redundant_copy(const char code[]) { + // Clear the error buffer.. + errout.str(""); + + Settings settings; + settings.addEnabled("performance"); + + // Tokenize.. + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + // Simplify token list.. + CheckOther checkOther(&tokenizer, &settings, this); + tokenizer.simplifyTokenList(); + checkOther.checkRedundantCopy(); + } + void checkRedundantCopy() { + check_redundant_copy("class A{public:A(){}};\n" + "const A& getA(){static A a;return a;}\n" + "int main()\n" + "{\n" + " const A a = getA();\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (performance) Use const reference for a to avoid unnecessary data copying.\n", errout.str()); + + check_redundant_copy("const int& getA(){static int a;return a;}\n" + "int main()\n" + "{\n" + " const int a = getA();\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_redundant_copy("const int& getA(){static int a;return a;}\n" + "int main()\n" + "{\n" + " int getA = 0;\n" + " const int a = getA + 3;\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_redundant_copy("class A{public:A(){}};\n" + "const A& getA(){static A a;return a;}\n" + "int main()\n" + "{\n" + " const A a(getA());\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (performance) Use const reference for a to avoid unnecessary data copying.\n", errout.str()); + + check_redundant_copy("const int& getA(){static int a;return a;}\n" + "int main()\n" + "{\n" + " const int a(getA());\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_redundant_copy("class A{\n" + "public:A(int a=0){_a = a;}\n" + "A operator+(const A & a){return A(_a+a._a);}\n" + "private:int _a;};\n" + "const A& getA(){static A a;return a;}\n" + "int main()\n" + "{\n" + " const A a = getA() + 1;\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_redundant_copy("class A{\n" + "public:A(int a=0){_a = a;}\n" + "A operator+(const A & a){return A(_a+a._a);}\n" + "private:int _a;};\n" + "const A& getA(){static A a;return a;}\n" + "int main()\n" + "{\n" + " const A a(getA()+1);\n" + " return 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void checkNegativeShift() { + check("void foo()\n" + "{\n" + " int a = 123;\n" + " a << -1;\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", 1() { - check("void f(int x) {\n" - " if ((x &&Foo a[5];\n" // Size of foo is unknown - " memset(a, 'a', 5) check("void f(int x) {\n" - " if (x < 1 && x > 1 - check("void f() {\n" - " char a[5];\n" - " memset(a, 'a', 5) check("void f(int x) {\n" - " if (x < 1 && x > 1 - check("void f() {\n" - " int a[5];\n" - " memset(a+15, 'a', 5) check("void f(int x) {\n" - " if (x < 1 && x > 1 - check("void f() {\n" - " bool a[5];\n" - " memset(a, false, 5*sizeof(bool)) check("void f(int x) {\n" - " if (x < 1 && x > 1 - check("void f() {\n" - " bool a[5];\n" - " memset(a, false, 5*sizeof(*a)) check("void f(int x) {\n" - " if (x < 1 && x > 1 - check("void f() {\n" - " bool a[5];\n" - " memset(a, false, 5);\n" + ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str()); + check("void foo()\n" + "{\n" + " int a = 123;\n" + " int i = -1;\n" + " a << i;\n" "}"); - ASSERT_EQUALS("[test.cpp:3]: (portability, inconclusive) Array 'a' might be filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5]: (error) Shifting by a negative value.\n", errout.str()); + check("void foo()\n" + "{\n" + " int a = 123;\n" + " a >> -1;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str()); + check("void foo()\n" + "{\n" + " int a = 123;\n" + " int i = -1;\n" + " a >> i;\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:5]: (error) Shifting by a negative value.\n", "", errout.str()); + check("void foo()\n" + "{\n" + " int a = 123;\n" + " a <<= -1;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str()); + check("void foo()\n" + "{\n" + " int a = 123;\n" + " a >>= -1;\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str()); } };