From 03cfe18c9bd49581c5b871030558a0e755f59ddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 14 Jan 2009 19:43:28 +0000 Subject: [PATCH] errmsg: Added 'condition is always true/false' --- src/checkother.cpp | 135 +++++++++++++++++++++++---------------------- src/cppcheck.cpp | 2 +- src/errormessage.h | 9 +++ test/testclass.cpp | 2 +- tools/errmsg.cpp | 1 + 5 files changed, 82 insertions(+), 67 deletions(-) diff --git a/src/checkother.cpp b/src/checkother.cpp index d20e82fc0..79e3f9b5e 100644 --- a/src/checkother.cpp +++ b/src/checkother.cpp @@ -194,87 +194,92 @@ void CheckOther::redundantCondition2() void CheckOther::WarningIf() { - // Search for 'if (condition);' - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + if (ErrorMessage::ifNoAction(_settings)) { - if (!Token::simpleMatch(tok, "if (")) - continue; - - // Search for the end paranthesis for the condition.. - int parlevel = 0; - for (const Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) + // Search for 'if (condition);' + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (tok2->str() == "(") - ++parlevel; - else if (tok2->str() == ")") + if (!Token::simpleMatch(tok, "if (")) + continue; + + // Search for the end paranthesis for the condition.. + int parlevel = 0; + for (const Token *tok2 = tok->next(); tok2; tok2 = tok2->next()) { - --parlevel; - if (parlevel <= 0) + if (tok2->str() == "(") + ++parlevel; + else if (tok2->str() == ")") { - if (Token::Match(tok2, ") ; !!else")) + --parlevel; + if (parlevel <= 0) { - _errorLogger->reportErr(ErrorMessage::ifNoAction(_tokenizer, tok)); + if (Token::Match(tok2, ") ; !!else")) + { + _errorLogger->reportErr(ErrorMessage::ifNoAction(_tokenizer, tok)); + } + break; } - break; } } } } - // Search for 'a=b; if (a==b)' - for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + if (ErrorMessage::conditionAlwaysTrueFalse(_settings)) { - // Begin statement? - if (! Token::Match(tok, "[;{}]")) - continue; - tok = tok->next(); - if (! tok) - break; - - if (!Token::Match(tok, "%var% = %var% ; if ( %var%")) - continue; - - if (strcmp(tok->strAt(9), ")") != 0) - continue; - - // var1 = var2 ; if ( var3 cond var4 ) - const char *var1 = tok->strAt(0); - const char *var2 = tok->strAt(2); - const char *var3 = tok->strAt(6); - const char *cond = tok->strAt(7); - const char *var4 = tok->strAt(8); - - // Check that var3 is equal with either var1 or var2 - if (strcmp(var1, var3) && strcmp(var2, var3)) - continue; - - // Check that var4 is equal with either var1 or var2 - if (strcmp(var1, var4) && strcmp(var2, var4)) - continue; - - // Check that there is a condition.. - const char *p[6] = {"==", "<=", ">=", "!=", "<", ">"}; - bool iscond = false; - for (int i = 0; i < 6; i++) + // Search for 'a=b; if (a==b)' + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (strcmp(cond, p[i]) == 0) - { - iscond = true; + // Begin statement? + if (! Token::Match(tok, "[;{}]")) + continue; + tok = tok->next(); + if (! tok) break; - } - } - if (!iscond) - break; - // we found the error. Report. - std::ostringstream ostr; - ostr << _tokenizer->fileLine(tok->tokAt(4)) << ": The condition is always "; - for (int i = 0; i < 6; i++) - { - if (strcmp(cond, p[i]) == 0) - ostr << (i < 3 ? "True" : "False"); + if (!Token::Match(tok, "%var% = %var% ; if ( %var%")) + continue; + + if (strcmp(tok->strAt(9), ")") != 0) + continue; + + // var1 = var2 ; if ( var3 cond var4 ) + const char *var1 = tok->strAt(0); + const char *var2 = tok->strAt(2); + const char *var3 = tok->strAt(6); + const char *cond = tok->strAt(7); + const char *var4 = tok->strAt(8); + + // Check that var3 is equal with either var1 or var2 + if (strcmp(var1, var3) && strcmp(var2, var3)) + continue; + + // Check that var4 is equal with either var1 or var2 + if (strcmp(var1, var4) && strcmp(var2, var4)) + continue; + + // Check that there is a condition.. + const char *p[6] = {"==", "<=", ">=", "!=", "<", ">"}; + bool iscond = false; + for (int i = 0; i < 6; i++) + { + if (strcmp(cond, p[i]) == 0) + { + iscond = true; + break; + } + } + if (!iscond) + break; + + // we found the error. Report. + bool b = false; + for (int i = 0; i < 6; i++) + { + if (strcmp(cond, p[i]) == 0) + b = (i < 3); + } + _errorLogger->reportErr(ErrorMessage::conditionAlwaysTrueFalse(_tokenizer, tok->tokAt(4), b ? "True" : "False")); } - _errorLogger->reportErr(ostr.str()); } } //--------------------------------------------------------------------------- diff --git a/src/cppcheck.cpp b/src/cppcheck.cpp index 115df1e6a..5b702ae3c 100644 --- a/src/cppcheck.cpp +++ b/src/cppcheck.cpp @@ -313,7 +313,7 @@ void CppCheck::checkFile(const std::string &code, const char FileName[]) checkClass.operatorEq(); // if (condition); - if (ErrorMessage::ifNoAction(_settings)) + if (ErrorMessage::ifNoAction(_settings) || ErrorMessage::conditionAlwaysTrueFalse(_settings)) checkOther.WarningIf(); // Unused struct members.. diff --git a/src/errormessage.h b/src/errormessage.h index b6295bd29..50237bcb2 100644 --- a/src/errormessage.h +++ b/src/errormessage.h @@ -318,5 +318,14 @@ public: return s._checkCodingStyle; } + static std::string conditionAlwaysTrueFalse(const Tokenizer *tokenizer, const Token *Location, const std::string &truefalse) + { + return msg1(tokenizer, Location) + "Condition is always " + truefalse + ""; + } + static bool conditionAlwaysTrueFalse(const Settings &s) + { + return s._checkCodingStyle; + } + }; #endif diff --git a/test/testclass.cpp b/test/testclass.cpp index 6162ee5b0..c4764b641 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -160,7 +160,7 @@ private: "\n" "private:\n" " ECODES _code;\n" - "};\n" ); + "};\n"); ASSERT_EQUALS("[test.cpp:10]: Uninitialized member variable 'Fred::_code'\n", errout.str()); } diff --git a/tools/errmsg.cpp b/tools/errmsg.cpp index 2f2335eb0..0e052ed5e 100644 --- a/tools/errmsg.cpp +++ b/tools/errmsg.cpp @@ -93,6 +93,7 @@ int main() err.push_back(Message("charBitOp", Message::style, "Warning - using char variable in bit operation")); err.push_back(Message("variableScope", Message::never, "The scope of the variable %1 can be limited", "varname")); err.push_back(Message("ifAssignment", Message::style, "Assignment in if-condition")); + err.push_back(Message("conditionAlwaysTrueFalse", Message::style, "Condition is always %1", "truefalse")); // Generate code.. std::cout << "Generate code.." << std::endl;