Skip to content

Commit 83c460f

Browse files
danmardanmar
committed
Fixed cppcheck-opensource#5017 (New check: division by zero, otherwise condition is redundant)
1 parent 40c5924 commit 83c460f

3 files changed

Lines changed: 143 additions & 0 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2155,6 +2155,74 @@ void CheckOther::zerodivError(const Token *tok)
21552155
reportError(tok, Severity::error, "zerodiv", "Division by zero.");
21562156
}
21572157

2158+
//---------------------------------------------------------------------------
2159+
void CheckOther::checkZeroDivisionOrUselessCondition()
2160+
{
2161+
if (!_settings->isEnabled("warning"))
2162+
return;
2163+
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
2164+
const std::size_t numberOfFunctions = symbolDatabase->functionScopes.size();
2165+
for (std::size_t functionIndex = 0; functionIndex < numberOfFunctions; ++functionIndex) {
2166+
const Scope * scope = symbolDatabase->functionScopes[functionIndex];
2167+
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
2168+
if (Token::Match(tok, "[/%] %var% !!.")) {
2169+
const unsigned int varid = tok->next()->varId();
2170+
const Variable *var = tok->next()->variable();
2171+
if (!var)
2172+
continue;
2173+
bool isVarUnsigned = var->typeEndToken()->isUnsigned();
2174+
for (const Token *typetok = var->typeStartToken(); typetok != var->typeEndToken(); typetok = typetok->next()) {
2175+
if (typetok->isUnsigned()) {
2176+
isVarUnsigned = true;
2177+
break;
2178+
}
2179+
}
2180+
for (const Token *tok2 = tok->tokAt(2); tok2; tok2 = tok2->next()) {
2181+
if (tok2->varId() == varid)
2182+
break;
2183+
if (tok2->str() == "}")
2184+
break;
2185+
if (Token::Match(tok2, "%var% (") && tok2->str() != "if" && (var->isGlobal() || !tok2->function()))
2186+
break;
2187+
if (isVarUnsigned && Token::Match(tok2, "(|%oror%|&& 0 < %varid% &&|%oror%|)", varid)) {
2188+
zerodivcondError(tok2,tok);
2189+
continue;
2190+
}
2191+
if (isVarUnsigned && Token::Match(tok2, "(|%oror%|&& 1 <= %varid% &&|%oror%|)", varid)) {
2192+
zerodivcondError(tok2,tok);
2193+
continue;
2194+
}
2195+
if (Token::Match(tok2, "(|%oror%|&& !| %varid% &&|%oror%|)", varid)) {
2196+
zerodivcondError(tok2,tok);
2197+
continue;
2198+
}
2199+
}
2200+
}
2201+
}
2202+
}
2203+
}
2204+
2205+
void CheckOther::zerodivcondError(const Token *tokcond, const Token *tokdiv)
2206+
{
2207+
std::list<const Token *> callstack;
2208+
while (Token::Match(tokcond, "(|%oror%|&&"))
2209+
tokcond = tokcond->next();
2210+
if (tokcond && tokdiv) {
2211+
callstack.push_back(tokcond);
2212+
callstack.push_back(tokdiv);
2213+
}
2214+
std::string condition;
2215+
if (Token::Match(tokcond, "%num% <|<=")) {
2216+
condition = tokcond->strAt(2) + ((tokcond->strAt(1) == "<") ? ">" : ">=") + tokcond->str();
2217+
} else if (tokcond) {
2218+
if (tokcond->str() == "!")
2219+
condition = tokcond->next()->str() + "==0";
2220+
else
2221+
condition = tokcond->str() + "!=0";
2222+
}
2223+
const std::string linenr(MathLib::longToString(tokdiv ? tokdiv->linenr() : 0));
2224+
reportError(callstack, Severity::warning, "zerodivcond", "Either the condition '"+condition+"' is useless or there is division by zero at line " + linenr + ".");
2225+
}
21582226
//---------------------------------------------------------------------------
21592227
//---------------------------------------------------------------------------
21602228

‎lib/checkother.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class CPPCHECKLIB CheckOther : public Check {
9090

9191
checkOther.invalidFunctionUsage();
9292
checkOther.checkZeroDivision();
93+
checkOther.checkZeroDivisionOrUselessCondition();
9394
checkOther.checkMathFunctions();
9495
checkOther.checkCCTypeFunctions();
9596

@@ -163,6 +164,9 @@ class CPPCHECKLIB CheckOther : public Check {
163164
/** @brief %Check zero division*/
164165
void checkZeroDivision();
165166

167+
/** @brief %Check zero division / useless condition */
168+
void checkZeroDivisionOrUselessCondition();
169+
166170
/** @brief Check for NaN (not-a-number) in an arithmetic expression */
167171
void checkNanInArithmeticExpression();
168172

@@ -286,6 +290,7 @@ class CPPCHECKLIB CheckOther : public Check {
286290
void variableScopeError(const Token *tok, const std::string &varname);
287291
void strPlusCharError(const Token *tok);
288292
void zerodivError(const Token *tok);
293+
void zerodivcondError(const Token *tokcond, const Token *tokdiv);
289294
void nanInArithmeticExpressionError(const Token *tok);
290295
void mathfunctionCallError(const Token *tok, const unsigned int numParam = 1);
291296
void cctypefunctionCallError(const Token *tok, const std::string &functionName, const std::string &value);
@@ -332,6 +337,7 @@ class CPPCHECKLIB CheckOther : public Check {
332337
c.sprintfOverlappingDataError(0, "varname");
333338
c.udivError(0, false);
334339
c.zerodivError(0);
340+
c.zerodivcondError(0,0);
335341
c.mathfunctionCallError(0);
336342
c.misusedScopeObjectError(NULL, "varname");
337343
c.doubleFreeError(0, "varname");
@@ -411,6 +417,9 @@ class CPPCHECKLIB CheckOther : public Check {
411417
"* cast the return values of getc(),fgetc() and getchar() to character and compare it to EOF\n"
412418
"* provide too big sleep times on POSIX systems\n"
413419

420+
// warning
421+
"* either division by zero or useless condition\n"
422+
414423
//performance
415424
"* redundant data copying for const variable\n"
416425
"* subsequent assignment or copying to a variable or buffer\n"

‎test/testother.cpp‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ class TestOther : public TestFixture {
4444
TEST_CASE(zeroDiv6);
4545
TEST_CASE(zeroDiv7); // #4930
4646

47+
TEST_CASE(zeroDivCond); // division by zero / useless condition
48+
4749
TEST_CASE(nanInArithmeticExpression);
4850

4951
TEST_CASE(sprintf1); // Dangerous usage of sprintf
@@ -428,6 +430,70 @@ class TestOther : public TestFixture {
428430
"[test.cpp:3]: (error) Division by zero.\n", errout.str());
429431
}
430432

433+
void zeroDivCond() {
434+
check("void f(unsigned int x) {\n"
435+
" int y = 17 / x;\n"
436+
" if (x > 0) {}\n"
437+
"}");
438+
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());
439+
440+
check("void f(unsigned int x) {\n"
441+
" int y = 17 / x;\n"
442+
" if (x >= 1) {}\n"
443+
"}");
444+
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());
445+
446+
check("void f(int x) {\n"
447+
" int y = 17 / x;\n"
448+
" if (x == 0) {}\n"
449+
"}");
450+
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());
451+
452+
check("void f(unsigned int x) {\n"
453+
" int y = 17 / x;\n"
454+
" if (x != 0) {}\n"
455+
"}");
456+
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());
457+
458+
// avoid false positives when variable is changed after division
459+
check("void f() {\n"
460+
" unsigned int x = do_something();\n"
461+
" int y = 17 / x;\n"
462+
" x = some+calculation;\n"
463+
" if (x != 0) {}\n"
464+
"}");
465+
ASSERT_EQUALS("", errout.str());
466+
467+
{
468+
// function is called that might modify global variable
469+
check("void do_something();\n"
470+
"int x;\n"
471+
"void f() {\n"
472+
" int y = 17 / x;\n"
473+
" do_something();\n"
474+
" if (x != 0) {}\n"
475+
"}");
476+
ASSERT_EQUALS("", errout.str());
477+
478+
// function is called. but don't care, variable is local
479+
check("void do_something();\n"
480+
"void f() {\n"
481+
" int x = some + calculation;\n"
482+
" int y = 17 / x;\n"
483+
" do_something();\n"
484+
" if (x != 0) {}\n"
485+
"}");
486+
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());
487+
}
488+
489+
check("int x;\n"
490+
"void f() {\n"
491+
" int y = 17 / x;\n"
492+
" while (y || x == 0) { x--; }\n"
493+
"}");
494+
ASSERT_EQUALS("", errout.str());
495+
}
496+
431497
void nanInArithmeticExpression() {
432498
check("void f()\n"
433499
"{\n"

0 commit comments

Comments
 (0)