From 6790d91fbb9ffdd6133152a4f5404e29c612c902 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 29 Jul 2015 19:54:57 +0200 Subject: [PATCH] Improve error messages for conditional values. make valueFlowSwitchVariable values conditional that depend on the case. Partial fix for #6884. --- lib/checkbufferoverrun.cpp | 42 +++++++----- lib/checknullpointer.cpp | 2 +- lib/checkother.cpp | 2 +- lib/valueflow.cpp | 19 ++++++ lib/valueflow.h | 4 ++ test/testbufferoverrun.cpp | 11 +++- test/testnullpointer.cpp | 127 ++++++++++++++++--------------------- test/testother.cpp | 14 ++-- test/testvalueflow.cpp | 26 +++++++- 9 files changed, 145 insertions(+), 102 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index eaf1d05b1..1dac99f22 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -65,20 +65,6 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector &index) { - std::ostringstream errmsg; - - errmsg << "Array '" << arrayInfo.varname(); - for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) - errmsg << "[" << arrayInfo.num(i) << "]"; - if (index.size() == 1) - errmsg << "' accessed at index " << index[0].intvalue << ", which is out of bounds."; - else { - errmsg << "' index " << arrayInfo.varname(); - for (std::size_t i = 0; i < index.size(); ++i) - errmsg << "[" << index[i].intvalue << "]"; - errmsg << " out of bounds."; - } - const Token *condition = nullptr; for (std::size_t i = 0; i < index.size(); ++i) { if (condition == nullptr) @@ -88,12 +74,38 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra if (condition != nullptr) { if (!_settings->isEnabled("warning")) return; - errmsg << " Otherwise condition '" << condition->expressionString() << "' is redundant."; + + std::ostringstream errmsg; + errmsg << ValueFlow::eitherTheConditionIsRedundant(condition) << " or the array '" << arrayInfo.varname(); + for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) + errmsg << "[" << arrayInfo.num(i) << "]"; + if (index.size() == 1) + errmsg << "' is accessed at index " << index[0].intvalue << ", which is out of bounds."; + else { + errmsg << "' index " << arrayInfo.varname(); + for (std::size_t i = 0; i < index.size(); ++i) + errmsg << "[" << index[i].intvalue << "]"; + errmsg << " is out of bounds."; + } + std::list callstack; callstack.push_back(tok); callstack.push_back(condition); reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), 0U, false); } else { + std::ostringstream errmsg; + errmsg << "Array '" << arrayInfo.varname(); + for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) + errmsg << "[" << arrayInfo.num(i) << "]"; + if (index.size() == 1) + errmsg << "' accessed at index " << index[0].intvalue << ", which is out of bounds."; + else { + errmsg << "' index " << arrayInfo.varname(); + for (std::size_t i = 0; i < index.size(); ++i) + errmsg << "[" << index[i].intvalue << "]"; + errmsg << " out of bounds."; + } + reportError(tok, Severity::error, "arrayIndexOutOfBounds", errmsg.str()); } } diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 220913eb1..b5e0f28d6 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -477,6 +477,6 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var std::list callstack; callstack.push_back(tok); callstack.push_back(nullCheck); - const std::string errmsg("Possible null pointer dereference: " + varname + " - otherwise it is redundant to check it against null."); + const std::string errmsg(ValueFlow::eitherTheConditionIsRedundant(nullCheck) + " or there is possible null pointer dereference: " + varname + "."); reportError(callstack, Severity::warning, "nullPointerRedundantCheck", errmsg, 0U, inconclusive); } diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 4b6e506ea..d10541c38 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1836,7 +1836,7 @@ void CheckOther::zerodivcondError(const Token *tokcond, const Token *tokdiv, boo } const std::string condition(tokcond ? tokcond->expressionString() : ""); const std::string linenr(MathLib::toString(tokdiv ? tokdiv->linenr() : 0)); - reportError(callstack, Severity::warning, "zerodivcond", "Either the condition '"+condition+"' is useless or there is division by zero at line " + linenr + ".", 0U, inconclusive); + reportError(callstack, Severity::warning, "zerodivcond", ValueFlow::eitherTheConditionIsRedundant(tokcond) + " or there is division by zero at line " + linenr + ".", 0U, inconclusive); } //--------------------------------------------------------------------------- diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 8884db7ae..bf7e341a6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2034,11 +2034,13 @@ static void valueFlowSwitchVariable(TokenList *tokenlist, SymbolDatabase* symbol if (Token::Match(tok, "case %num% :")) { std::list values; values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str()))); + values.back().condition = tok; while (Token::Match(tok->tokAt(3), ";| case %num% :")) { tok = tok->tokAt(3); if (!tok->isName()) tok = tok->next(); values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str()))); + values.back().condition = tok; } for (std::list::const_iterator val = values.begin(); val != values.end(); ++val) { valueFlowReverse(tokenlist, @@ -2222,3 +2224,20 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowSubFunction(tokenlist, errorLogger, settings); valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings); } + + +std::string ValueFlow::eitherTheConditionIsRedundant(const Token *condition) +{ + if (!condition) + return "Either the condition is redundant"; + if (condition->str() == "case") { + std::string expr; + for (const Token *tok = condition; tok && tok->str() != ":"; tok = tok->next()) { + expr += tok->str(); + if (Token::Match(tok, "%name%|%num% %name%|%num%")) + expr += ' '; + } + return "Either the switch case '" + expr + "' is redundant"; + } + return "Either the condition '" + condition->expressionString() + "' is redundant"; +} diff --git a/lib/valueflow.h b/lib/valueflow.h index 529b9408a..9587eeed3 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -21,6 +21,8 @@ #define valueflowH //--------------------------------------------------------------------------- +#include + class Token; class TokenList; class SymbolDatabase; @@ -92,6 +94,8 @@ namespace ValueFlow { }; void setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings); + + std::string eitherTheConditionIsRedundant(const Token *condition); } #endif // valueflowH diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index c775b0bc9..c1760a5d3 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -2033,7 +2033,16 @@ private: " str[i] = 0;\n" " if (i==10) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Array 'str[3]' accessed at index 10, which is out of bounds. Otherwise condition 'i==10' is redundant.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition 'i==10' is redundant or the array 'str[3]' is accessed at index 10, which is out of bounds.\n", errout.str()); + + check("void f(int i) {\n" + " char str[3];\n" + " str[i] = 0;\n" + " switch (i) {\n" + " case 10: break;\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Either the switch case 'case 10' is redundant or the array 'str[3]' is accessed at index 10, which is out of bounds.\n", errout.str()); check("void f() {\n" " char str[3];\n" diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 06a695899..e2ed18678 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -123,7 +123,7 @@ private: " while (tok);\n" " tok = tok->next();\n" "}", true); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning, inconclusive) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning, inconclusive) Either the condition 'tok' is redundant or there is possible null pointer dereference: tok.\n", errout.str()); // #2681 { @@ -140,7 +140,7 @@ private: ASSERT_EQUALS("", errout.str()); check(code, true); // inconclusive=true => error - ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:3]: (warning, inconclusive) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:3]: (warning, inconclusive) Either the condition 'tok' is redundant or there is possible null pointer dereference: tok.\n", errout.str()); } check("void foo()\n" @@ -151,7 +151,7 @@ private: " tok = tok->next();\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Possible null pointer dereference: tok - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Either the condition 'while' is redundant or there is possible null pointer dereference: tok.\n", errout.str()); check("void foo(Token &tok)\n" "{\n" @@ -277,7 +277,7 @@ private: " if (!abc)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition '!abc' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); check("void foo(struct ABC *abc) {\n" " bar(abc->a);\n" @@ -286,9 +286,9 @@ private: " if (!abc)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n" - "[test.cpp:3] -> [test.cpp:5]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n" - "[test.cpp:4] -> [test.cpp:5]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (warning) Either the condition '!abc' is redundant or there is possible null pointer dereference: abc.\n" + "[test.cpp:3] -> [test.cpp:5]: (warning) Either the condition '!abc' is redundant or there is possible null pointer dereference: abc.\n" + "[test.cpp:4] -> [test.cpp:5]: (warning) Either the condition '!abc' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); check("void foo(ABC *abc) {\n" " if (abc->a == 3) {\n" @@ -296,7 +296,7 @@ private: " }\n" " if (abc) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (warning) Either the condition 'if(abc)' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); check("void f(ABC *abc) {\n" " if (abc->x == 0) {\n" @@ -304,7 +304,7 @@ private: " }\n" " if (!abc);\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (warning) Either the condition '!abc' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); // TODO: False negative if member of member is dereferenced check("void foo(ABC *abc) {\n" @@ -319,7 +319,7 @@ private: " if (abc && abc->b == 0)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Either the condition 'if(abc&&abc.b==0)' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); // ok dereferencing in a condition check("void foo(struct ABC *abc)\n" @@ -442,7 +442,7 @@ private: " do_stuff();\n" " if (abc) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n",errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Either the condition 'if(abc)' is redundant or there is possible null pointer dereference: abc.\n",errout.str()); // #2641 - local pointer, function call check("void f(ABC *abc) {\n" @@ -450,7 +450,7 @@ private: " do_stuff();\n" " if (abc) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n",errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Either the condition 'if(abc)' is redundant or there is possible null pointer dereference: abc.\n",errout.str()); // #2691 - switch/break check("void f(ABC *abc) {\n" @@ -499,7 +499,7 @@ private: check(code); ASSERT_EQUALS("", errout.str()); check(code, true); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning, inconclusive) Possible null pointer dereference: fred - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning, inconclusive) Either the condition 'if(fred)' is redundant or there is possible null pointer dereference: fred.\n", errout.str()); } // #3425 - false positives when there are macros @@ -519,21 +519,21 @@ private: " if (!p)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(int *p)\n" "{\n" " *p = 0;\n" " if (p) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition 'if(p)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(int *p)\n" "{\n" " *p = 0;\n" " if (p || q) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition 'if(p||q)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(int *p)\n" "{\n" @@ -541,7 +541,7 @@ private: " if (!p)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(char *p)\n" "{\n" @@ -549,14 +549,14 @@ private: " if (!p)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(char *p)\n" "{\n" " if (*p == 0) { }\n" " if (!p) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // no error check("void foo()\n" @@ -609,7 +609,7 @@ private: " if (!p)\n" " ;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // while check("void f(int *p) {\n" @@ -622,7 +622,7 @@ private: " *p = 0;\n" " while (p) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Either the condition 'while(p)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // Ticket #3125 check("void foo(ABC *p)\n" @@ -699,7 +699,7 @@ private: " assert(p && (*p<=6));\n" " if (p) { *p = 0; }\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Either the condition 'if(p)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(x *p)\n" "{\n" @@ -764,7 +764,7 @@ private: " a = b ? c : d;\n" " if (item) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Possible null pointer dereference: item - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning) Either the condition 'if(item)' is redundant or there is possible null pointer dereference: item.\n", errout.str()); check("BOOL GotoFlyAnchor()\n" // #2243 "{\n" @@ -1045,7 +1045,7 @@ private: " p = q;\n" " if (p || *p) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:5]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:5]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); } } @@ -1347,48 +1347,27 @@ private: " }\n" " *p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); - - check("void foo(char *p) {\n" - " if (NULL == p) {\n" - " }\n" - " *p = 0;\n" - "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); - - check("void foo(char *p) {\n" - " if (p == NULL) {\n" - " }\n" - " *p = 0;\n" - "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); - - check("void foo(char *p) {\n" - " if (p == NULL) {\n" - " }\n" - " printf(\"%c\", *p);\n" - "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(char *p) {\n" " if (p && *p == 0) {\n" " }\n" " printf(\"%c\", *p);\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(char *p) {\n" " if (p && *p == 0) {\n" " } else { *p = 0; }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(char *p) {\n" " if (p) {\n" " }\n" " strcpy(p, \"abc\");\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(char *p) {\n" " if (p) {\n" @@ -1396,7 +1375,7 @@ private: " bar();\n" " strcpy(p, \"abc\");\n" "}"); - ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:2]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void foo(abc *p) {\n" " if (!p) {\n" @@ -1465,8 +1444,8 @@ private: " return 5+*p;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n" - "[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n" + "[test.cpp:4] -> [test.cpp:2]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // operator! check("void f() {\n" @@ -1502,7 +1481,7 @@ private: " }\n" " *p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:2]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // #2467 - unknown macro may terminate the application check("void f(Fred *fred) {\n" @@ -1605,7 +1584,7 @@ private: " if (fred) { }\n" " return fred->a;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: fred - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'fred' is redundant or there is possible null pointer dereference: fred.\n", errout.str()); // #2789 - assign and check pointer check("void f() {\n" @@ -1613,7 +1592,7 @@ private: " if (!p) { }\n" " *p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // check, assign and use check("void f() {\n" @@ -1640,7 +1619,7 @@ private: " return;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // check, and use check("void f() {\n" @@ -1649,7 +1628,7 @@ private: " return;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // check, and use check("void f() {\n" @@ -1667,7 +1646,7 @@ private: " return;\n" " }\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:3]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // check, and use check("void f(struct X *p, int x) {\n" @@ -1687,7 +1666,7 @@ private: ASSERT_EQUALS("", errout.str()); check(code, true); // inconclusive - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning, inconclusive) Possible null pointer dereference: fred - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning, inconclusive) Either the condition 'fred==0' is redundant or there is possible null pointer dereference: fred.\n", errout.str()); } check("void f(char *s) {\n" // #3358 @@ -1720,7 +1699,7 @@ private: " if (!p) {}\n" " return q ? p->x : 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("int f(ABC *p) {\n" // FP : return && " if (!p) {}\n" @@ -1732,7 +1711,7 @@ private: " if (x || !p) {}\n" " *p = 0;\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition '!p' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // sizeof check("void f() {\n" @@ -2062,7 +2041,7 @@ private: "}", false); ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p\n" "[test.cpp:4]: (error) Possible null pointer dereference: p\n" - "[test.cpp:6] -> [test.cpp:5]: (warning) Possible null pointer dereference: q - otherwise it is redundant to check it against null.\n", errout.str()); + "[test.cpp:6] -> [test.cpp:5]: (warning) Either the condition 'q==0' is redundant or there is possible null pointer dereference: q.\n", errout.str()); check("void f(const char* p) {\n" " if(p == 0) {\n" @@ -2072,12 +2051,12 @@ private: " std::cout << abc << p;\n" " }\n" "}", false); - TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n" - "[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n" - "[test.cpp:5] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n" - "[test.cpp:6] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", - "[test.cpp:3] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n" - "[test.cpp:4] -> [test.cpp:2]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", + TODO_ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n" + "[test.cpp:4] -> [test.cpp:2]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n" + "[test.cpp:5] -> [test.cpp:2]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n" + "[test.cpp:6] -> [test.cpp:2]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n", + "[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n" + "[test.cpp:4] -> [test.cpp:2]: (warning) Either the condition 'p==0' is redundant or there is possible null pointer dereference: p.\n", errout.str()); check("void f() {\n" @@ -2141,7 +2120,7 @@ private: " foo(p);\n" " if (p) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Either the condition 'if(p)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // function seen (taking reference parameter) check("void foo(int *&p) { }\n" @@ -2161,7 +2140,7 @@ private: " foo(p);\n" " if (p) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Either the condition 'if(p)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); // inconclusive check("void f(int *p) {\n" @@ -2169,7 +2148,7 @@ private: " foo(p);\n" " if (p) { }\n" "}", true); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning, inconclusive) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning, inconclusive) Either the condition 'if(p)' is redundant or there is possible null pointer dereference: p.\n", errout.str()); } // dereference struct pointer and then check if it's null @@ -2190,7 +2169,7 @@ private: " foo(abc);\n" " if (abc) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Either the condition 'if(abc)' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); // function implementation not seen check("void foo(struct ABC *abc);\n" @@ -2200,7 +2179,7 @@ private: " foo(abc);\n" " if (abc) { }\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Either the condition 'if(abc)' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); // inconclusive check("void f(struct ABC *abc) {\n" @@ -2208,7 +2187,7 @@ private: " foo(abc);\n" " if (abc) { }\n" "}", true); - ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning, inconclusive) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (warning, inconclusive) Either the condition 'if(abc)' is redundant or there is possible null pointer dereference: abc.\n", errout.str()); } } diff --git a/test/testother.cpp b/test/testother.cpp index 750840f90..dc50a3379 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -422,25 +422,25 @@ private: " int y = 17 / x;\n" " if (x > 0) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x>0' is useless or there is division by zero at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x>0' is redundant or there is division by zero at line 2.\n", errout.str()); check("void f(unsigned int x) {\n" " int y = 17 / x;\n" " if (x >= 1) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x>=1' is useless or there is division by zero at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x>=1' is redundant or there is division by zero at line 2.\n", errout.str()); check("void f(int x) {\n" " int y = 17 / x;\n" " if (x == 0) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x==0' is useless or there is division by zero at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x==0' is redundant or there is division by zero at line 2.\n", errout.str()); check("void f(unsigned int x) {\n" " int y = 17 / x;\n" " if (x != 0) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x!=0' is useless or there is division by zero at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'x!=0' is redundant or there is division by zero at line 2.\n", errout.str()); // function call check("void f1(int x, int y) { c=x/y; }\n" @@ -448,7 +448,7 @@ private: " f1(123,y);\n" " if (y>0){}\n" "}"); - ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Either the condition 'y>0' is useless or there is division by zero at line 1.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1]: (warning) Either the condition 'y>0' is redundant or there is division by zero at line 1.\n", errout.str()); // avoid false positives when variable is changed after division check("void f() {\n" @@ -478,7 +478,7 @@ private: " do_something();\n" " if (x != 0) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:4]: (warning) Either the condition 'x!=0' is useless or there is division by zero at line 4.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:4]: (warning) Either the condition 'x!=0' is redundant or there is division by zero at line 4.\n", errout.str()); } check("void do_something(int value);\n" @@ -516,7 +516,7 @@ private: " int r = (a?b:c) / d;\n" " if (d == 0) {}\n" "}"); - ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'd==0' is useless or there is division by zero at line 2.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Either the condition 'd==0' is redundant or there is division by zero at line 2.\n", errout.str()); check("int f(int a) {\n" " int r = a ? 1 / a : 0;\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 496e8c05a..fdcf50db1 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -128,6 +128,26 @@ private: return false; } + bool testConditionalValueOfX(const char code[], unsigned int linenr, int value) { + // Tokenize.. + Settings settings; + Tokenizer tokenizer(&settings, this); + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); + + for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { + if (tok->str() == "x" && tok->linenr() == linenr) { + std::list::const_iterator it; + for (it = tok->values.begin(); it != tok->values.end(); ++it) { + if (it->intvalue == value && !it->tokvalue && it->condition) + return true; + } + } + } + + return false; + } + void bailout(const char code[]) { Settings settings; settings.debugwarnings = true; @@ -1278,9 +1298,9 @@ private: " };\n" " a = x;\n" // <- x can be 14 "}"; - ASSERT_EQUALS(true, testValueOfX(code, 2U, 14)); - ASSERT_EQUALS(true, testValueOfX(code, 4U, 14)); - ASSERT_EQUALS(true, testValueOfX(code, 6U, 14)); + ASSERT_EQUALS(true, testConditionalValueOfX(code, 2U, 14)); + ASSERT_EQUALS(true, testConditionalValueOfX(code, 4U, 14)); + ASSERT_EQUALS(true, testConditionalValueOfX(code, 6U, 14)); } void valueFlowForLoop() {