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

Improve error messages for conditional values. make valueFlowSwitchVariable values conditional that depend on the case. Partial fix for #6884.

This commit is contained in:
Daniel Marjamäki 2015-07-29 19:54:57 +02:00
parent 738057229c
commit 6790d91fbb
9 changed files with 145 additions and 102 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
lib/checkbufferoverrun.cpp
View File

@ -65,20 +65,6 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra
void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &index) void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &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; const Token *condition = nullptr;
for (std::size_t i = 0; i < index.size(); ++i) { for (std::size_t i = 0; i < index.size(); ++i) {
if (condition == nullptr) if (condition == nullptr)
@ -88,12 +74,38 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra
if (condition != nullptr) { if (condition != nullptr) {
if (!_settings->isEnabled("warning")) if (!_settings->isEnabled("warning"))
return; 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<const Token *> callstack; std::list<const Token *> callstack;
callstack.push_back(tok); callstack.push_back(tok);
callstack.push_back(condition); callstack.push_back(condition);
reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), 0U, false); reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), 0U, false);
} else { } 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()); reportError(tok, Severity::error, "arrayIndexOutOfBounds", errmsg.str());
} }
} }

2
lib/checknullpointer.cpp
View File

@ -477,6 +477,6 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var
std::list<const Token*> callstack; std::list<const Token*> callstack;
callstack.push_back(tok); callstack.push_back(tok);
callstack.push_back(nullCheck); 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); reportError(callstack, Severity::warning, "nullPointerRedundantCheck", errmsg, 0U, inconclusive);
} }

2
lib/checkother.cpp
View File

@ -1836,7 +1836,7 @@ void CheckOther::zerodivcondError(const Token *tokcond, const Token *tokdiv, boo
} }
const std::string condition(tokcond ? tokcond->expressionString() : ""); const std::string condition(tokcond ? tokcond->expressionString() : "");
const std::string linenr(MathLib::toString(tokdiv ? tokdiv->linenr() : 0)); 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);
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------

19
lib/valueflow.cpp
View File

@ -2034,11 +2034,13 @@ static void valueFlowSwitchVariable(TokenList *tokenlist, SymbolDatabase* symbol
if (Token::Match(tok, "case %num% :")) { if (Token::Match(tok, "case %num% :")) {
std::list<ValueFlow::Value> values; std::list<ValueFlow::Value> values;
values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str()))); values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str())));
values.back().condition = tok;
while (Token::Match(tok->tokAt(3), ";| case %num% :")) { while (Token::Match(tok->tokAt(3), ";| case %num% :")) {
tok = tok->tokAt(3); tok = tok->tokAt(3);
if (!tok->isName()) if (!tok->isName())
tok = tok->next(); tok = tok->next();
values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str()))); values.push_back(ValueFlow::Value(MathLib::toLongNumber(tok->next()->str())));
values.back().condition = tok;
} }
for (std::list<ValueFlow::Value>::const_iterator val = values.begin(); val != values.end(); ++val) { for (std::list<ValueFlow::Value>::const_iterator val = values.begin(); val != values.end(); ++val) {
valueFlowReverse(tokenlist, valueFlowReverse(tokenlist,
@ -2222,3 +2224,20 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowSubFunction(tokenlist, errorLogger, settings); valueFlowSubFunction(tokenlist, errorLogger, settings);
valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, 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";
}

4
lib/valueflow.h
View File

@ -21,6 +21,8 @@
#define valueflowH #define valueflowH
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
#include <string>
class Token; class Token;
class TokenList; class TokenList;
class SymbolDatabase; class SymbolDatabase;
@ -92,6 +94,8 @@ namespace ValueFlow {
}; };
void setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings); void setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings);
std::string eitherTheConditionIsRedundant(const Token *condition);
} }
#endif // valueflowH #endif // valueflowH

11
test/testbufferoverrun.cpp
View File

@ -2033,7 +2033,16 @@ private:
" str[i] = 0;\n" " str[i] = 0;\n"
" if (i==10) {}\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" check("void f() {\n"
" char str[3];\n" " char str[3];\n"

127
test/testnullpointer.cpp
View File

@ -123,7 +123,7 @@ private:
" while (tok);\n" " while (tok);\n"
" tok = tok->next();\n" " tok = tok->next();\n"
"}", true); "}", 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 // #2681
{ {
@ -140,7 +140,7 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check(code, true); // inconclusive=true => error 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" check("void foo()\n"
@ -151,7 +151,7 @@ private:
" tok = tok->next();\n" " tok = tok->next();\n"
" }\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" check("void foo(Token &tok)\n"
"{\n" "{\n"
@ -277,7 +277,7 @@ private:
" if (!abc)\n" " if (!abc)\n"
" ;\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" check("void foo(struct ABC *abc) {\n"
" bar(abc->a);\n" " bar(abc->a);\n"
@ -286,9 +286,9 @@ private:
" if (!abc)\n" " if (!abc)\n"
" ;\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" 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) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\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) Possible null pointer dereference: abc - otherwise it is redundant to check it against null.\n", errout.str()); "[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" check("void foo(ABC *abc) {\n"
" if (abc->a == 3) {\n" " if (abc->a == 3) {\n"
@ -296,7 +296,7 @@ private:
" }\n" " }\n"
" if (abc) {}\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" check("void f(ABC *abc) {\n"
" if (abc->x == 0) {\n" " if (abc->x == 0) {\n"
@ -304,7 +304,7 @@ private:
" }\n" " }\n"
" if (!abc);\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 // TODO: False negative if member of member is dereferenced
check("void foo(ABC *abc) {\n" check("void foo(ABC *abc) {\n"
@ -319,7 +319,7 @@ private:
" if (abc && abc->b == 0)\n" " if (abc && abc->b == 0)\n"
" ;\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 // ok dereferencing in a condition
check("void foo(struct ABC *abc)\n" check("void foo(struct ABC *abc)\n"
@ -442,7 +442,7 @@ private:
" do_stuff();\n" " do_stuff();\n"
" if (abc) { }\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 // #2641 - local pointer, function call
check("void f(ABC *abc) {\n" check("void f(ABC *abc) {\n"
@ -450,7 +450,7 @@ private:
" do_stuff();\n" " do_stuff();\n"
" if (abc) { }\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 // #2691 - switch/break
check("void f(ABC *abc) {\n" check("void f(ABC *abc) {\n"
@ -499,7 +499,7 @@ private:
check(code); check(code);
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check(code, true); 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 // #3425 - false positives when there are macros
@ -519,21 +519,21 @@ private:
" if (!p)\n" " if (!p)\n"
" ;\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" check("void foo(int *p)\n"
"{\n" "{\n"
" *p = 0;\n" " *p = 0;\n"
" if (p) { }\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" check("void foo(int *p)\n"
"{\n" "{\n"
" *p = 0;\n" " *p = 0;\n"
" if (p || q) { }\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" check("void foo(int *p)\n"
"{\n" "{\n"
@ -541,7 +541,7 @@ private:
" if (!p)\n" " if (!p)\n"
" ;\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" check("void foo(char *p)\n"
"{\n" "{\n"
@ -549,14 +549,14 @@ private:
" if (!p)\n" " if (!p)\n"
" ;\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" check("void foo(char *p)\n"
"{\n" "{\n"
" if (*p == 0) { }\n" " if (*p == 0) { }\n"
" if (!p) { }\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 // no error
check("void foo()\n" check("void foo()\n"
@ -609,7 +609,7 @@ private:
" if (!p)\n" " if (!p)\n"
" ;\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 // while
check("void f(int *p) {\n" check("void f(int *p) {\n"
@ -622,7 +622,7 @@ private:
" *p = 0;\n" " *p = 0;\n"
" while (p) { }\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 // Ticket #3125
check("void foo(ABC *p)\n" check("void foo(ABC *p)\n"
@ -699,7 +699,7 @@ private:
" assert(p && (*p<=6));\n" " assert(p && (*p<=6));\n"
" if (p) { *p = 0; }\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" check("void foo(x *p)\n"
"{\n" "{\n"
@ -764,7 +764,7 @@ private:
" a = b ? c : d;\n" " a = b ? c : d;\n"
" if (item) { }\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 check("BOOL GotoFlyAnchor()\n" // #2243
"{\n" "{\n"
@ -1045,7 +1045,7 @@ private:
" p = q;\n" " p = q;\n"
" if (p || *p) { }\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" " }\n"
" *p = 0;\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()); 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 (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());
check("void foo(char *p) {\n" check("void foo(char *p) {\n"
" if (p && *p == 0) {\n" " if (p && *p == 0) {\n"
" }\n" " }\n"
" printf(\"%c\", *p);\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" check("void foo(char *p) {\n"
" if (p && *p == 0) {\n" " if (p && *p == 0) {\n"
" } else { *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" check("void foo(char *p) {\n"
" if (p) {\n" " if (p) {\n"
" }\n" " }\n"
" strcpy(p, \"abc\");\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" check("void foo(char *p) {\n"
" if (p) {\n" " if (p) {\n"
@ -1396,7 +1375,7 @@ private:
" bar();\n" " bar();\n"
" strcpy(p, \"abc\");\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" check("void foo(abc *p) {\n"
" if (!p) {\n" " if (!p) {\n"
@ -1465,8 +1444,8 @@ private:
" return 5+*p;\n" " return 5+*p;\n"
" }\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" 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) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\n", errout.str()); "[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! // operator!
check("void f() {\n" check("void f() {\n"
@ -1502,7 +1481,7 @@ private:
" }\n" " }\n"
" *p = 0;\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 // #2467 - unknown macro may terminate the application
check("void f(Fred *fred) {\n" check("void f(Fred *fred) {\n"
@ -1605,7 +1584,7 @@ private:
" if (fred) { }\n" " if (fred) { }\n"
" return fred->a;\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 // #2789 - assign and check pointer
check("void f() {\n" check("void f() {\n"
@ -1613,7 +1592,7 @@ private:
" if (!p) { }\n" " if (!p) { }\n"
" *p = 0;\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, assign and use
check("void f() {\n" check("void f() {\n"
@ -1640,7 +1619,7 @@ private:
" return;\n" " return;\n"
" }\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, and use
check("void f() {\n" check("void f() {\n"
@ -1649,7 +1628,7 @@ private:
" return;\n" " return;\n"
" }\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, and use
check("void f() {\n" check("void f() {\n"
@ -1667,7 +1646,7 @@ private:
" return;\n" " return;\n"
" }\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, and use
check("void f(struct X *p, int x) {\n" check("void f(struct X *p, int x) {\n"
@ -1687,7 +1666,7 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check(code, true); // inconclusive 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 check("void f(char *s) {\n" // #3358
@ -1720,7 +1699,7 @@ private:
" if (!p) {}\n" " if (!p) {}\n"
" return q ? p->x : 0;\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 && check("int f(ABC *p) {\n" // FP : return &&
" if (!p) {}\n" " if (!p) {}\n"
@ -1732,7 +1711,7 @@ private:
" if (x || !p) {}\n" " if (x || !p) {}\n"
" *p = 0;\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 // sizeof
check("void f() {\n" check("void f() {\n"
@ -2062,7 +2041,7 @@ private:
"}", false); "}", false);
ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p\n" ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: p\n"
"[test.cpp:4]: (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" check("void f(const char* p) {\n"
" if(p == 0) {\n" " if(p == 0) {\n"
@ -2072,12 +2051,12 @@ private:
" std::cout << abc << p;\n" " std::cout << abc << p;\n"
" }\n" " }\n"
"}", false); "}", 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" 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) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\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) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\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) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\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) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\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) Possible null pointer dereference: p - otherwise it is redundant to check it against null.\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()); errout.str());
check("void f() {\n" check("void f() {\n"
@ -2141,7 +2120,7 @@ private:
" foo(p);\n" " foo(p);\n"
" if (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) // function seen (taking reference parameter)
check("void foo(int *&p) { }\n" check("void foo(int *&p) { }\n"
@ -2161,7 +2140,7 @@ private:
" foo(p);\n" " foo(p);\n"
" if (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 // inconclusive
check("void f(int *p) {\n" check("void f(int *p) {\n"
@ -2169,7 +2148,7 @@ private:
" foo(p);\n" " foo(p);\n"
" if (p) { }\n" " if (p) { }\n"
"}", true); "}", 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 // dereference struct pointer and then check if it's null
@ -2190,7 +2169,7 @@ private:
" foo(abc);\n" " foo(abc);\n"
" if (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 // function implementation not seen
check("void foo(struct ABC *abc);\n" check("void foo(struct ABC *abc);\n"
@ -2200,7 +2179,7 @@ private:
" foo(abc);\n" " foo(abc);\n"
" if (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 // inconclusive
check("void f(struct ABC *abc) {\n" check("void f(struct ABC *abc) {\n"
@ -2208,7 +2187,7 @@ private:
" foo(abc);\n" " foo(abc);\n"
" if (abc) { }\n" " if (abc) { }\n"
"}", true); "}", 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());
} }
} }

14
test/testother.cpp
View File

@ -422,25 +422,25 @@ private:
" int y = 17 / x;\n" " int y = 17 / x;\n"
" if (x > 0) {}\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" check("void f(unsigned int x) {\n"
" int y = 17 / x;\n" " int y = 17 / x;\n"
" if (x >= 1) {}\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" check("void f(int x) {\n"
" int y = 17 / x;\n" " int y = 17 / x;\n"
" if (x == 0) {}\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" check("void f(unsigned int x) {\n"
" int y = 17 / x;\n" " int y = 17 / x;\n"
" if (x != 0) {}\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 // function call
check("void f1(int x, int y) { c=x/y; }\n" check("void f1(int x, int y) { c=x/y; }\n"
@ -448,7 +448,7 @@ private:
" f1(123,y);\n" " f1(123,y);\n"
" if (y>0){}\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 // avoid false positives when variable is changed after division
check("void f() {\n" check("void f() {\n"
@ -478,7 +478,7 @@ private:
" do_something();\n" " do_something();\n"
" if (x != 0) {}\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" check("void do_something(int value);\n"
@ -516,7 +516,7 @@ private:
" int r = (a?b:c) / d;\n" " int r = (a?b:c) / d;\n"
" if (d == 0) {}\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" check("int f(int a) {\n"
" int r = a ? 1 / a : 0;\n" " int r = a ? 1 / a : 0;\n"

26
test/testvalueflow.cpp
View File

@ -128,6 +128,26 @@ private:
return false; 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<ValueFlow::Value>::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[]) { void bailout(const char code[]) {
Settings settings; Settings settings;
settings.debugwarnings = true; settings.debugwarnings = true;
@ -1278,9 +1298,9 @@ private:
" };\n" " };\n"
" a = x;\n" // <- x can be 14 " a = x;\n" // <- x can be 14
"}"; "}";
ASSERT_EQUALS(true, testValueOfX(code, 2U, 14)); ASSERT_EQUALS(true, testConditionalValueOfX(code, 2U, 14));
ASSERT_EQUALS(true, testValueOfX(code, 4U, 14)); ASSERT_EQUALS(true, testConditionalValueOfX(code, 4U, 14));
ASSERT_EQUALS(true, testValueOfX(code, 6U, 14)); ASSERT_EQUALS(true, testConditionalValueOfX(code, 6U, 14));
} }
void valueFlowForLoop() { void valueFlowForLoop() {