Moved several condition checks from checkOther to checkCondition (former checkAssignIf)
This commit is contained in:
parent
6a5eda51d3
commit
06a92e8981
|
@ -1,332 +0,0 @@
|
|||
/*
|
||||
* Cppcheck - A tool for static C/C++ code analysis
|
||||
* Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Check for assignment / condition mismatches
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
#include "checkassignif.h"
|
||||
#include "symboldatabase.h"
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
// Register this check class (by creating a static instance of it)
|
||||
namespace {
|
||||
CheckAssignIf instance;
|
||||
}
|
||||
|
||||
|
||||
void CheckAssignIf::assignIf()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
||||
if (tok->str() != "=")
|
||||
continue;
|
||||
|
||||
if (Token::Match(tok->tokAt(-2), "[;{}] %var% =")) {
|
||||
const Variable *var = tok->previous()->variable();
|
||||
if (var == 0)
|
||||
continue;
|
||||
|
||||
char bitop = '\0';
|
||||
MathLib::bigint num = 0;
|
||||
|
||||
if (Token::Match(tok->next(), "%num% [&|]")) {
|
||||
bitop = tok->strAt(2).at(0);
|
||||
num = MathLib::toLongNumber(tok->next()->str());
|
||||
} else {
|
||||
const Token *endToken = Token::findsimplematch(tok, ";");
|
||||
|
||||
// Casting address
|
||||
if (endToken && Token::Match(endToken->tokAt(-4), "* ) & %any% ;"))
|
||||
endToken = nullptr;
|
||||
|
||||
if (endToken && Token::Match(endToken->tokAt(-2), "[&|] %num% ;")) {
|
||||
bitop = endToken->strAt(-2).at(0);
|
||||
num = MathLib::toLongNumber(endToken->previous()->str());
|
||||
}
|
||||
}
|
||||
|
||||
if (bitop == '\0')
|
||||
continue;
|
||||
|
||||
if (num < 0 && bitop == '|')
|
||||
continue;
|
||||
|
||||
assignIfParseScope(tok, tok->tokAt(4), var->declarationId(), var->isLocal(), bitop, num);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** parse scopes recursively */
|
||||
bool CheckAssignIf::assignIfParseScope(const Token * const assignTok,
|
||||
const Token * const startTok,
|
||||
const unsigned int varid,
|
||||
const bool islocal,
|
||||
const char bitop,
|
||||
const MathLib::bigint num)
|
||||
{
|
||||
bool ret = false;
|
||||
|
||||
for (const Token *tok2 = startTok; tok2; tok2 = tok2->next()) {
|
||||
if (Token::Match(tok2->tokAt(2), "%varid% %cop% %num% ;", varid) && tok2->strAt(3) == std::string(1U, bitop)) {
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(tok2->strAt(4));
|
||||
if ((bitop == '&') && (0 == (num & num2)))
|
||||
mismatchingBitAndError(assignTok, num, tok2, num2);
|
||||
}
|
||||
if (Token::Match(tok2, "%varid% =", varid)) {
|
||||
if (Token::Match(tok2->tokAt(2), "%varid% %cop% %num% ;", varid) && tok2->strAt(3) == std::string(1U, bitop)) {
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(tok2->strAt(4));
|
||||
if ((bitop == '&') && (0 == (num & num2)))
|
||||
mismatchingBitAndError(assignTok, num, tok2, num2);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
if (Token::Match(tok2, "++|-- %varid%", varid) || Token::Match(tok2, "%varid% ++|--", varid))
|
||||
return true;
|
||||
if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) {
|
||||
unsigned int argumentNumber = 0;
|
||||
const Token *ftok;
|
||||
for (ftok = tok2; ftok && ftok->str() != "("; ftok = ftok->previous()) {
|
||||
if (ftok->str() == ")")
|
||||
ftok = ftok->link();
|
||||
else if (ftok->str() == ",")
|
||||
argumentNumber++;
|
||||
}
|
||||
ftok = ftok ? ftok->previous() : nullptr;
|
||||
if (!(ftok && ftok->function()))
|
||||
return true;
|
||||
const Variable *par = ftok->function()->getArgumentVar(argumentNumber);
|
||||
if (par == nullptr || par->isReference() || par->isPointer())
|
||||
return true;
|
||||
}
|
||||
if (tok2->str() == "}")
|
||||
return false;
|
||||
if (Token::Match(tok2, "break|continue|return"))
|
||||
ret = true;
|
||||
if (ret && tok2->str() == ";")
|
||||
return false;
|
||||
if (!islocal && Token::Match(tok2, "%var% (") && !Token::simpleMatch(tok2->next()->link(), ") {"))
|
||||
return true;
|
||||
if (Token::Match(tok2, "if|while (")) {
|
||||
if (!islocal && tok2->str() == "while")
|
||||
continue;
|
||||
|
||||
// parse condition
|
||||
const Token * const end = tok2->next()->link();
|
||||
for (; tok2 != end; tok2 = tok2->next()) {
|
||||
if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) {
|
||||
return true;
|
||||
}
|
||||
if (Token::Match(tok2,"&&|%oror%|( %varid% %any% %num% &&|%oror%|)", varid)) {
|
||||
const Token *vartok = tok2->next();
|
||||
const std::string& op(vartok->strAt(1));
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2));
|
||||
const std::string condition(vartok->str() + op + vartok->strAt(2));
|
||||
if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num))
|
||||
assignIfError(assignTok, tok2, condition, false);
|
||||
else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num))
|
||||
assignIfError(assignTok, tok2, condition, true);
|
||||
}
|
||||
}
|
||||
|
||||
bool ret1 = assignIfParseScope(assignTok, end->tokAt(2), varid, islocal, bitop, num);
|
||||
bool ret2 = false;
|
||||
if (Token::simpleMatch(end->next()->link(), "} else {"))
|
||||
ret2 = assignIfParseScope(assignTok, end->next()->link()->tokAt(3), varid, islocal, bitop, num);
|
||||
if (ret1 || ret2)
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
void CheckAssignIf::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result)
|
||||
{
|
||||
std::list<const Token *> locations;
|
||||
locations.push_back(tok1);
|
||||
locations.push_back(tok2);
|
||||
|
||||
reportError(locations,
|
||||
Severity::style,
|
||||
"assignIfError",
|
||||
"Mismatching assignment and comparison, comparison '" + condition + "' is always " + std::string(result ? "true" : "false") + ".");
|
||||
}
|
||||
|
||||
|
||||
void CheckAssignIf::mismatchingBitAndError(const Token *tok1, const MathLib::bigint num1, const Token *tok2, const MathLib::bigint num2)
|
||||
{
|
||||
std::list<const Token *> locations;
|
||||
locations.push_back(tok1);
|
||||
locations.push_back(tok2);
|
||||
|
||||
std::ostringstream msg;
|
||||
msg << "Mismatching bitmasks. Result is always 0 ("
|
||||
<< "X = Y & 0x" << std::hex << num1 << "; Z = X & 0x" << std::hex << num2 << "; => Z=0).";
|
||||
|
||||
reportError(locations,
|
||||
Severity::style,
|
||||
"mismatchingBitAnd",
|
||||
msg.str());
|
||||
}
|
||||
|
||||
|
||||
static void getnumchildren(const Token *tok, std::list<MathLib::bigint> &numchildren)
|
||||
{
|
||||
if (tok->astOperand1() && tok->astOperand1()->isNumber())
|
||||
numchildren.push_back(MathLib::toLongNumber(tok->astOperand1()->str()));
|
||||
else if (tok->astOperand1() && tok->str() == tok->astOperand1()->str())
|
||||
getnumchildren(tok->astOperand1(), numchildren);
|
||||
if (tok->astOperand2() && tok->astOperand2()->isNumber())
|
||||
numchildren.push_back(MathLib::toLongNumber(tok->astOperand2()->str()));
|
||||
else if (tok->astOperand2() && tok->str() == tok->astOperand2()->str())
|
||||
getnumchildren(tok->astOperand2(), numchildren);
|
||||
}
|
||||
|
||||
void CheckAssignIf::comparison()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
// Experimental code based on AST
|
||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
||||
if (Token::Match(tok, "==|!=")) {
|
||||
const Token *expr1 = tok->astOperand1();
|
||||
const Token *expr2 = tok->astOperand2();
|
||||
if (!expr1 || !expr2)
|
||||
continue;
|
||||
if (expr1->isNumber())
|
||||
std::swap(expr1,expr2);
|
||||
if (!expr2->isNumber())
|
||||
continue;
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(expr2->str());
|
||||
if (num2 < 0)
|
||||
continue;
|
||||
if (!Token::Match(expr1,"[&|]"))
|
||||
continue;
|
||||
std::list<MathLib::bigint> numbers;
|
||||
getnumchildren(expr1, numbers);
|
||||
for (std::list<MathLib::bigint>::const_iterator num = numbers.begin(); num != numbers.end(); ++num) {
|
||||
const MathLib::bigint num1 = *num;
|
||||
if (num1 < 0)
|
||||
continue;
|
||||
if ((expr1->str() == "&" && (num1 & num2) != num2) ||
|
||||
(expr1->str() == "|" && (num1 | num2) != num2)) {
|
||||
const std::string& op(tok->str());
|
||||
comparisonError(expr1, expr1->str(), num1, op, num2, op=="==" ? false : true);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckAssignIf::comparisonError(const Token *tok, const std::string &bitop, MathLib::bigint value1, const std::string &op, MathLib::bigint value2, bool result)
|
||||
{
|
||||
std::ostringstream expression;
|
||||
expression << std::hex << "(X " << bitop << " 0x" << value1 << ") " << op << " 0x" << value2;
|
||||
|
||||
const std::string errmsg("Expression '" + expression.str() + "' is always " + (result?"true":"false") + ".\n"
|
||||
"The expression '" + expression.str() + "' is always " + (result?"true":"false") +
|
||||
". Check carefully constants and operators used, these errors might be hard to "
|
||||
"spot sometimes. In case of complex expression it might help to split it to "
|
||||
"separate expressions.");
|
||||
|
||||
reportError(tok, Severity::style, "comparisonError", errmsg);
|
||||
}
|
||||
|
||||
extern bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
|
||||
|
||||
static bool isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
||||
{
|
||||
if (!cond1 || !cond2)
|
||||
return false;
|
||||
|
||||
// same expressions
|
||||
if (isSameExpression(cond1,cond2,constFunctions))
|
||||
return true;
|
||||
|
||||
// bitwise overlap for example 'x&7' and 'x==1'
|
||||
if (cond1->str() == "&" && cond1->astOperand1() && cond2->astOperand2()) {
|
||||
const Token *expr1 = cond1->astOperand1();
|
||||
const Token *num1 = cond1->astOperand2();
|
||||
if (!num1) // unary operator&
|
||||
return false;
|
||||
if (!num1->isNumber())
|
||||
std::swap(expr1,num1);
|
||||
if (!num1->isNumber() || MathLib::isNegative(num1->str()))
|
||||
return false;
|
||||
|
||||
if (!Token::Match(cond2, "&|==") || !cond2->astOperand1() || !cond2->astOperand2())
|
||||
return false;
|
||||
const Token *expr2 = cond2->astOperand1();
|
||||
const Token *num2 = cond2->astOperand2();
|
||||
if (!num2->isNumber())
|
||||
std::swap(expr2,num2);
|
||||
if (!num2->isNumber() || MathLib::isNegative(num2->str()))
|
||||
return false;
|
||||
|
||||
if (!isSameExpression(expr1,expr2,constFunctions))
|
||||
return false;
|
||||
|
||||
const MathLib::bigint value1 = MathLib::toLongNumber(num1->str());
|
||||
const MathLib::bigint value2 = MathLib::toLongNumber(num2->str());
|
||||
return ((value1 & value2) == value2);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
void CheckAssignIf::multiCondition()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
|
||||
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
|
||||
if (i->type != Scope::eIf || !Token::simpleMatch(i->classDef, "if ("))
|
||||
continue;
|
||||
|
||||
const Token * const cond1 = i->classDef->next()->astOperand2();
|
||||
|
||||
const Token * tok2 = i->classDef->next();
|
||||
while (tok2) {
|
||||
tok2 = tok2->link();
|
||||
if (!Token::simpleMatch(tok2, ") {"))
|
||||
break;
|
||||
tok2 = tok2->linkAt(1);
|
||||
if (!Token::simpleMatch(tok2, "} else { if ("))
|
||||
break;
|
||||
tok2 = tok2->tokAt(4);
|
||||
|
||||
if (isOverlappingCond(cond1, tok2->astOperand2(), _settings->library.functionpure))
|
||||
multiConditionError(tok2, cond1->linenr());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckAssignIf::multiConditionError(const Token *tok, unsigned int line1)
|
||||
{
|
||||
std::ostringstream errmsg;
|
||||
errmsg << "Expression is always false because 'else if' condition matches previous condition at line "
|
||||
<< line1 << ".";
|
||||
|
||||
reportError(tok, Severity::style, "multiCondition", errmsg.str());
|
||||
}
|
|
@ -0,0 +1,855 @@
|
|||
/*
|
||||
* Cppcheck - A tool for static C/C++ code analysis
|
||||
* Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Check for condition mismatches
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
#include "checkcondition.h"
|
||||
#include "checkother.h"
|
||||
#include "symboldatabase.h"
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
// Register this check class (by creating a static instance of it)
|
||||
namespace {
|
||||
CheckCondition instance;
|
||||
}
|
||||
|
||||
|
||||
void CheckCondition::assignIf()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
||||
if (tok->str() != "=")
|
||||
continue;
|
||||
|
||||
if (Token::Match(tok->tokAt(-2), "[;{}] %var% =")) {
|
||||
const Variable *var = tok->previous()->variable();
|
||||
if (var == 0)
|
||||
continue;
|
||||
|
||||
char bitop = '\0';
|
||||
MathLib::bigint num = 0;
|
||||
|
||||
if (Token::Match(tok->next(), "%num% [&|]")) {
|
||||
bitop = tok->strAt(2).at(0);
|
||||
num = MathLib::toLongNumber(tok->next()->str());
|
||||
} else {
|
||||
const Token *endToken = Token::findsimplematch(tok, ";");
|
||||
|
||||
// Casting address
|
||||
if (endToken && Token::Match(endToken->tokAt(-4), "* ) & %any% ;"))
|
||||
endToken = nullptr;
|
||||
|
||||
if (endToken && Token::Match(endToken->tokAt(-2), "[&|] %num% ;")) {
|
||||
bitop = endToken->strAt(-2).at(0);
|
||||
num = MathLib::toLongNumber(endToken->previous()->str());
|
||||
}
|
||||
}
|
||||
|
||||
if (bitop == '\0')
|
||||
continue;
|
||||
|
||||
if (num < 0 && bitop == '|')
|
||||
continue;
|
||||
|
||||
assignIfParseScope(tok, tok->tokAt(4), var->declarationId(), var->isLocal(), bitop, num);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** parse scopes recursively */
|
||||
bool CheckCondition::assignIfParseScope(const Token * const assignTok,
|
||||
const Token * const startTok,
|
||||
const unsigned int varid,
|
||||
const bool islocal,
|
||||
const char bitop,
|
||||
const MathLib::bigint num)
|
||||
{
|
||||
bool ret = false;
|
||||
|
||||
for (const Token *tok2 = startTok; tok2; tok2 = tok2->next()) {
|
||||
if (Token::Match(tok2->tokAt(2), "%varid% %cop% %num% ;", varid) && tok2->strAt(3) == std::string(1U, bitop)) {
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(tok2->strAt(4));
|
||||
if ((bitop == '&') && (0 == (num & num2)))
|
||||
mismatchingBitAndError(assignTok, num, tok2, num2);
|
||||
}
|
||||
if (Token::Match(tok2, "%varid% =", varid)) {
|
||||
if (Token::Match(tok2->tokAt(2), "%varid% %cop% %num% ;", varid) && tok2->strAt(3) == std::string(1U, bitop)) {
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(tok2->strAt(4));
|
||||
if ((bitop == '&') && (0 == (num & num2)))
|
||||
mismatchingBitAndError(assignTok, num, tok2, num2);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
if (Token::Match(tok2, "++|-- %varid%", varid) || Token::Match(tok2, "%varid% ++|--", varid))
|
||||
return true;
|
||||
if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) {
|
||||
unsigned int argumentNumber = 0;
|
||||
const Token *ftok;
|
||||
for (ftok = tok2; ftok && ftok->str() != "("; ftok = ftok->previous()) {
|
||||
if (ftok->str() == ")")
|
||||
ftok = ftok->link();
|
||||
else if (ftok->str() == ",")
|
||||
argumentNumber++;
|
||||
}
|
||||
ftok = ftok ? ftok->previous() : nullptr;
|
||||
if (!(ftok && ftok->function()))
|
||||
return true;
|
||||
const Variable *par = ftok->function()->getArgumentVar(argumentNumber);
|
||||
if (par == nullptr || par->isReference() || par->isPointer())
|
||||
return true;
|
||||
}
|
||||
if (tok2->str() == "}")
|
||||
return false;
|
||||
if (Token::Match(tok2, "break|continue|return"))
|
||||
ret = true;
|
||||
if (ret && tok2->str() == ";")
|
||||
return false;
|
||||
if (!islocal && Token::Match(tok2, "%var% (") && !Token::simpleMatch(tok2->next()->link(), ") {"))
|
||||
return true;
|
||||
if (Token::Match(tok2, "if|while (")) {
|
||||
if (!islocal && tok2->str() == "while")
|
||||
continue;
|
||||
|
||||
// parse condition
|
||||
const Token * const end = tok2->next()->link();
|
||||
for (; tok2 != end; tok2 = tok2->next()) {
|
||||
if (Token::Match(tok2, "[(,] &| %varid% [,)]", varid)) {
|
||||
return true;
|
||||
}
|
||||
if (Token::Match(tok2,"&&|%oror%|( %varid% %any% %num% &&|%oror%|)", varid)) {
|
||||
const Token *vartok = tok2->next();
|
||||
const std::string& op(vartok->strAt(1));
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(vartok->strAt(2));
|
||||
const std::string condition(vartok->str() + op + vartok->strAt(2));
|
||||
if (op == "==" && (num & num2) != ((bitop=='&') ? num2 : num))
|
||||
assignIfError(assignTok, tok2, condition, false);
|
||||
else if (op == "!=" && (num & num2) != ((bitop=='&') ? num2 : num))
|
||||
assignIfError(assignTok, tok2, condition, true);
|
||||
}
|
||||
}
|
||||
|
||||
bool ret1 = assignIfParseScope(assignTok, end->tokAt(2), varid, islocal, bitop, num);
|
||||
bool ret2 = false;
|
||||
if (Token::simpleMatch(end->next()->link(), "} else {"))
|
||||
ret2 = assignIfParseScope(assignTok, end->next()->link()->tokAt(3), varid, islocal, bitop, num);
|
||||
if (ret1 || ret2)
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
void CheckCondition::assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result)
|
||||
{
|
||||
std::list<const Token *> locations;
|
||||
locations.push_back(tok1);
|
||||
locations.push_back(tok2);
|
||||
|
||||
reportError(locations,
|
||||
Severity::style,
|
||||
"assignIfError",
|
||||
"Mismatching assignment and comparison, comparison '" + condition + "' is always " + std::string(result ? "true" : "false") + ".");
|
||||
}
|
||||
|
||||
|
||||
void CheckCondition::mismatchingBitAndError(const Token *tok1, const MathLib::bigint num1, const Token *tok2, const MathLib::bigint num2)
|
||||
{
|
||||
std::list<const Token *> locations;
|
||||
locations.push_back(tok1);
|
||||
locations.push_back(tok2);
|
||||
|
||||
std::ostringstream msg;
|
||||
msg << "Mismatching bitmasks. Result is always 0 ("
|
||||
<< "X = Y & 0x" << std::hex << num1 << "; Z = X & 0x" << std::hex << num2 << "; => Z=0).";
|
||||
|
||||
reportError(locations,
|
||||
Severity::style,
|
||||
"mismatchingBitAnd",
|
||||
msg.str());
|
||||
}
|
||||
|
||||
|
||||
static void getnumchildren(const Token *tok, std::list<MathLib::bigint> &numchildren)
|
||||
{
|
||||
if (tok->astOperand1() && tok->astOperand1()->isNumber())
|
||||
numchildren.push_back(MathLib::toLongNumber(tok->astOperand1()->str()));
|
||||
else if (tok->astOperand1() && tok->str() == tok->astOperand1()->str())
|
||||
getnumchildren(tok->astOperand1(), numchildren);
|
||||
if (tok->astOperand2() && tok->astOperand2()->isNumber())
|
||||
numchildren.push_back(MathLib::toLongNumber(tok->astOperand2()->str()));
|
||||
else if (tok->astOperand2() && tok->str() == tok->astOperand2()->str())
|
||||
getnumchildren(tok->astOperand2(), numchildren);
|
||||
}
|
||||
|
||||
void CheckCondition::comparison()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
// Experimental code based on AST
|
||||
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
||||
if (Token::Match(tok, "==|!=")) {
|
||||
const Token *expr1 = tok->astOperand1();
|
||||
const Token *expr2 = tok->astOperand2();
|
||||
if (!expr1 || !expr2)
|
||||
continue;
|
||||
if (expr1->isNumber())
|
||||
std::swap(expr1,expr2);
|
||||
if (!expr2->isNumber())
|
||||
continue;
|
||||
const MathLib::bigint num2 = MathLib::toLongNumber(expr2->str());
|
||||
if (num2 < 0)
|
||||
continue;
|
||||
if (!Token::Match(expr1,"[&|]"))
|
||||
continue;
|
||||
std::list<MathLib::bigint> numbers;
|
||||
getnumchildren(expr1, numbers);
|
||||
for (std::list<MathLib::bigint>::const_iterator num = numbers.begin(); num != numbers.end(); ++num) {
|
||||
const MathLib::bigint num1 = *num;
|
||||
if (num1 < 0)
|
||||
continue;
|
||||
if ((expr1->str() == "&" && (num1 & num2) != num2) ||
|
||||
(expr1->str() == "|" && (num1 | num2) != num2)) {
|
||||
const std::string& op(tok->str());
|
||||
comparisonError(expr1, expr1->str(), num1, op, num2, op=="==" ? false : true);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::comparisonError(const Token *tok, const std::string &bitop, MathLib::bigint value1, const std::string &op, MathLib::bigint value2, bool result)
|
||||
{
|
||||
std::ostringstream expression;
|
||||
expression << std::hex << "(X " << bitop << " 0x" << value1 << ") " << op << " 0x" << value2;
|
||||
|
||||
const std::string errmsg("Expression '" + expression.str() + "' is always " + (result?"true":"false") + ".\n"
|
||||
"The expression '" + expression.str() + "' is always " + (result?"true":"false") +
|
||||
". Check carefully constants and operators used, these errors might be hard to "
|
||||
"spot sometimes. In case of complex expression it might help to split it to "
|
||||
"separate expressions.");
|
||||
|
||||
reportError(tok, Severity::style, "comparisonError", errmsg);
|
||||
}
|
||||
|
||||
extern bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
|
||||
|
||||
static bool isOverlappingCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
||||
{
|
||||
if (!cond1 || !cond2)
|
||||
return false;
|
||||
|
||||
// same expressions
|
||||
if (isSameExpression(cond1,cond2,constFunctions))
|
||||
return true;
|
||||
|
||||
// bitwise overlap for example 'x&7' and 'x==1'
|
||||
if (cond1->str() == "&" && cond1->astOperand1() && cond2->astOperand2()) {
|
||||
const Token *expr1 = cond1->astOperand1();
|
||||
const Token *num1 = cond1->astOperand2();
|
||||
if (!num1) // unary operator&
|
||||
return false;
|
||||
if (!num1->isNumber())
|
||||
std::swap(expr1,num1);
|
||||
if (!num1->isNumber() || MathLib::isNegative(num1->str()))
|
||||
return false;
|
||||
|
||||
if (!Token::Match(cond2, "&|==") || !cond2->astOperand1() || !cond2->astOperand2())
|
||||
return false;
|
||||
const Token *expr2 = cond2->astOperand1();
|
||||
const Token *num2 = cond2->astOperand2();
|
||||
if (!num2->isNumber())
|
||||
std::swap(expr2,num2);
|
||||
if (!num2->isNumber() || MathLib::isNegative(num2->str()))
|
||||
return false;
|
||||
|
||||
if (!isSameExpression(expr1,expr2,constFunctions))
|
||||
return false;
|
||||
|
||||
const MathLib::bigint value1 = MathLib::toLongNumber(num1->str());
|
||||
const MathLib::bigint value2 = MathLib::toLongNumber(num2->str());
|
||||
return ((value1 & value2) == value2);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
void CheckCondition::multiCondition()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
|
||||
for (std::list<Scope>::const_iterator i = symbolDatabase->scopeList.begin(); i != symbolDatabase->scopeList.end(); ++i) {
|
||||
if (i->type != Scope::eIf || !Token::simpleMatch(i->classDef, "if ("))
|
||||
continue;
|
||||
|
||||
const Token * const cond1 = i->classDef->next()->astOperand2();
|
||||
|
||||
const Token * tok2 = i->classDef->next();
|
||||
while (tok2) {
|
||||
tok2 = tok2->link();
|
||||
if (!Token::simpleMatch(tok2, ") {"))
|
||||
break;
|
||||
tok2 = tok2->linkAt(1);
|
||||
if (!Token::simpleMatch(tok2, "} else { if ("))
|
||||
break;
|
||||
tok2 = tok2->tokAt(4);
|
||||
|
||||
if (isOverlappingCond(cond1, tok2->astOperand2(), _settings->library.functionpure))
|
||||
multiConditionError(tok2, cond1->linenr());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::multiConditionError(const Token *tok, unsigned int line1)
|
||||
{
|
||||
std::ostringstream errmsg;
|
||||
errmsg << "Expression is always false because 'else if' condition matches previous condition at line "
|
||||
<< line1 << ".";
|
||||
|
||||
reportError(tok, Severity::style, "multiCondition", errmsg.str());
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Detect oppositing inner and outer conditions
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
static bool isOppositeCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
||||
{
|
||||
if (!cond1 || !cond2)
|
||||
return false;
|
||||
|
||||
if (cond1->str() == "!")
|
||||
return isSameExpression(cond1->astOperand1(), cond2, constFunctions);
|
||||
|
||||
if (cond2->str() == "!")
|
||||
return isSameExpression(cond1, cond2->astOperand1(), constFunctions);
|
||||
|
||||
if (!cond1->isComparisonOp() || !cond2->isComparisonOp())
|
||||
return false;
|
||||
|
||||
const std::string &comp1 = cond1->str();
|
||||
|
||||
// condition found .. get comparator
|
||||
std::string comp2;
|
||||
if (isSameExpression(cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
|
||||
isSameExpression(cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
|
||||
comp2 = cond2->str();
|
||||
} else if (isSameExpression(cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
|
||||
isSameExpression(cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
|
||||
comp2 = cond2->str();
|
||||
if (comp2[0] == '>')
|
||||
comp2[0] = '<';
|
||||
else if (comp2[0] == '<')
|
||||
comp2[0] = '>';
|
||||
}
|
||||
|
||||
// is condition opposite?
|
||||
return ((comp1 == "==" && comp2 == "!=") ||
|
||||
(comp1 == "!=" && comp2 == "==") ||
|
||||
(comp1 == "<" && comp2 == ">=") ||
|
||||
(comp1 == "<" && comp2 == ">") ||
|
||||
(comp1 == "<=" && comp2 == ">") ||
|
||||
(comp1 == ">" && comp2 == "<=") ||
|
||||
(comp1 == ">" && comp2 == "<") ||
|
||||
(comp1 == ">=" && comp2 == "<"));
|
||||
}
|
||||
|
||||
void CheckCondition::oppositeInnerCondition()
|
||||
{
|
||||
if (!_settings->isEnabled("warning"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
|
||||
for (std::list<Scope>::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
|
||||
if (scope->type != Scope::eIf)
|
||||
continue;
|
||||
|
||||
if (!Token::simpleMatch(scope->classDef->linkAt(1), ") {"))
|
||||
continue;
|
||||
|
||||
bool nonlocal = false; // nonlocal variable used in condition
|
||||
std::set<unsigned int> vars; // variables used in condition
|
||||
for (const Token *cond = scope->classDef; cond != scope->classDef->linkAt(1); cond = cond->next()) {
|
||||
if (cond->varId()) {
|
||||
vars.insert(cond->varId());
|
||||
const Variable *var = cond->variable();
|
||||
nonlocal |= (var && (!var->isLocal() || var->isStatic()) && !var->isArgument());
|
||||
// TODO: if var is pointer check what it points at
|
||||
nonlocal |= (var && (var->isPointer() || var->isReference()));
|
||||
} else if (cond->isName()) {
|
||||
// varid is 0. this is possibly a nonlocal variable..
|
||||
nonlocal |= (cond->astParent() && cond->astParent()->isConstOp());
|
||||
}
|
||||
}
|
||||
|
||||
// parse until inner condition is reached..
|
||||
const Token *ifToken = nullptr;
|
||||
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
|
||||
if (Token::simpleMatch(tok, "if (")) {
|
||||
ifToken = tok;
|
||||
break;
|
||||
}
|
||||
if (Token::Match(tok, "%type% (") && nonlocal) // function call -> bailout if there are nonlocal variables
|
||||
break;
|
||||
else if ((tok->varId() && vars.find(tok->varId()) != vars.end()) ||
|
||||
(!tok->varId() && nonlocal)) {
|
||||
if (Token::Match(tok, "%var% ++|--|="))
|
||||
break;
|
||||
if (Token::Match(tok, "%var% [")) {
|
||||
const Token *tok2 = tok->linkAt(1);
|
||||
while (Token::simpleMatch(tok2, "] ["))
|
||||
tok2 = tok2->linkAt(1);
|
||||
if (Token::simpleMatch(tok2, "] ="))
|
||||
break;
|
||||
}
|
||||
if (Token::Match(tok->previous(), "++|--|& %var%"))
|
||||
break;
|
||||
if (Token::Match(tok->previous(), "[(,] %var% [,)]")) {
|
||||
// is variable unchanged? default is false..
|
||||
bool unchanged = false;
|
||||
|
||||
// locate start parentheses in function call..
|
||||
unsigned int argumentNumber = 0;
|
||||
const Token *start = tok->previous();
|
||||
while (start && start->str() == ",") {
|
||||
start = start->astParent();
|
||||
++argumentNumber;
|
||||
}
|
||||
|
||||
start = start ? start->previous() : nullptr;
|
||||
if (start && start->function()) {
|
||||
const Variable *arg = start->function()->getArgumentVar(argumentNumber);
|
||||
if (arg && !arg->isPointer() && !arg->isReference())
|
||||
unchanged = true;
|
||||
}
|
||||
|
||||
if (!unchanged)
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!ifToken)
|
||||
continue;
|
||||
|
||||
// Condition..
|
||||
const Token *cond1 = scope->classDef->next()->astOperand2();
|
||||
const Token *cond2 = ifToken->next()->astOperand2();
|
||||
|
||||
if (isOppositeCond(cond1, cond2, _settings->library.functionpure))
|
||||
oppositeInnerConditionError(scope->classDef, cond2);
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::oppositeInnerConditionError(const Token *tok1, const Token* tok2)
|
||||
{
|
||||
std::list<const Token*> callstack;
|
||||
callstack.push_back(tok1);
|
||||
callstack.push_back(tok2);
|
||||
reportError(callstack, Severity::warning, "oppositeInnerCondition", "Opposite conditions in nested 'if' blocks lead to a dead code block.");
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// if ((x != 1) || (x != 3)) // expression always true
|
||||
// if ((x == 1) && (x == 3)) // expression always false
|
||||
// if ((x < 1) && (x > 3)) // expression always false
|
||||
// if ((x > 3) || (x < 10)) // expression always true
|
||||
// if ((x > 5) && (x != 1)) // second comparison always true
|
||||
//
|
||||
// Check for suspect logic for an expression consisting of 2 comparison
|
||||
// expressions with a shared variable and constants and a logical operator
|
||||
// between them.
|
||||
//
|
||||
// Suggest a different logical operator when the logical operator between
|
||||
// the comparisons is probably wrong.
|
||||
//
|
||||
// Inform that second comparison is always true when first comparison is true.
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
static std::string invertOperatorForOperandSwap(std::string s)
|
||||
{
|
||||
if (s[0] == '<')
|
||||
s[0] = '>';
|
||||
else if (s[0] == '>')
|
||||
s[0] = '<';
|
||||
return s;
|
||||
}
|
||||
|
||||
static bool checkIntRelation(const std::string &op, const MathLib::bigint value1, const MathLib::bigint value2)
|
||||
{
|
||||
return (op == "==" && value1 == value2) ||
|
||||
(op == "!=" && value1 != value2) ||
|
||||
(op == ">" && value1 > value2) ||
|
||||
(op == ">=" && value1 >= value2) ||
|
||||
(op == "<" && value1 < value2) ||
|
||||
(op == "<=" && value1 <= value2);
|
||||
}
|
||||
|
||||
static bool checkFloatRelation(const std::string &op, const double value1, const double value2)
|
||||
{
|
||||
return (op == ">" && value1 > value2) ||
|
||||
(op == ">=" && value1 >= value2) ||
|
||||
(op == "<" && value1 < value2) ||
|
||||
(op == "<=" && value1 <= value2);
|
||||
}
|
||||
|
||||
template<class T> static T getvalue(const int test, const T value1, const T value2)
|
||||
{
|
||||
// test:
|
||||
// 1 => return value that is less than both value1 and value2
|
||||
// 2 => return value1
|
||||
// 3 => return value that is between value1 and value2
|
||||
// 4 => return value2
|
||||
// 5 => return value that is larger than both value1 and value2
|
||||
switch (test) {
|
||||
case 1: {
|
||||
T ret = std::min(value1, value2);
|
||||
if ((ret - (T)1) < ret)
|
||||
return ret - (T)1;
|
||||
else if ((ret / (T)2) < ret)
|
||||
return ret / (T)2;
|
||||
else if ((ret * (T)2) < ret)
|
||||
return ret * (T)2;
|
||||
return ret;
|
||||
}
|
||||
case 2:
|
||||
return value1;
|
||||
case 3:
|
||||
return (value1 + value2) / (T)2;
|
||||
case 4:
|
||||
return value2;
|
||||
case 5: {
|
||||
T ret = std::max(value1, value2);
|
||||
if ((ret + (T)1) > ret)
|
||||
return ret + (T)1;
|
||||
else if ((ret / (T)2) > ret)
|
||||
return ret / (T)2;
|
||||
else if ((ret * (T)2) > ret)
|
||||
return ret * (T)2;
|
||||
return ret;
|
||||
}
|
||||
};
|
||||
return 0;
|
||||
}
|
||||
|
||||
void CheckCondition::checkIncorrectLogicOperator()
|
||||
{
|
||||
bool style = _settings->isEnabled("style");
|
||||
bool warning = _settings->isEnabled("warning");
|
||||
if (!style && !warning)
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t ii = 0; ii < functions; ++ii) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[ii];
|
||||
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
// Opposite comparisons
|
||||
if (Token::Match(tok, "%oror%|&&") &&
|
||||
tok->astOperand1() &&
|
||||
tok->astOperand2() &&
|
||||
(tok->astOperand1()->isName() || tok->astOperand2()->isName()) &&
|
||||
isOppositeCond(tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
|
||||
|
||||
const bool alwaysTrue(tok->str() == "||");
|
||||
incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue);
|
||||
}
|
||||
|
||||
else if (Token::Match(tok, "&&|%oror%")) {
|
||||
// Comparison #1 (LHS)
|
||||
const Token *comp1 = tok->astOperand1();
|
||||
if (comp1 && comp1->str() == tok->str())
|
||||
comp1 = comp1->astOperand2();
|
||||
|
||||
// Comparison #2 (RHS)
|
||||
const Token *comp2 = tok->astOperand2();
|
||||
|
||||
if (!comp1 || !comp1->isComparisonOp() || !comp1->astOperand1() || !comp1->astOperand2())
|
||||
continue;
|
||||
if (!comp2 || !comp2->isComparisonOp() || !comp2->astOperand1() || !comp2->astOperand2())
|
||||
continue;
|
||||
|
||||
std::string op1, value1;
|
||||
const Token *expr1;
|
||||
if (comp1->astOperand1()->isLiteral()) {
|
||||
op1 = invertOperatorForOperandSwap(comp1->str());
|
||||
value1 = comp1->astOperand1()->str();
|
||||
expr1 = comp1->astOperand2();
|
||||
} else if (comp1->astOperand2()->isLiteral()) {
|
||||
op1 = comp1->str();
|
||||
value1 = comp1->astOperand2()->str();
|
||||
expr1 = comp1->astOperand1();
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
std::string op2, value2;
|
||||
const Token *expr2;
|
||||
if (comp2->astOperand1()->isLiteral()) {
|
||||
op2 = invertOperatorForOperandSwap(comp2->str());
|
||||
value2 = comp2->astOperand1()->str();
|
||||
expr2 = comp2->astOperand2();
|
||||
} else if (comp2->astOperand2()->isLiteral()) {
|
||||
op2 = comp2->str();
|
||||
value2 = comp2->astOperand2()->str();
|
||||
expr2 = comp2->astOperand1();
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only float and int values are currently handled
|
||||
if (!MathLib::isInt(value1) && !MathLib::isFloat(value1))
|
||||
continue;
|
||||
if (!MathLib::isInt(value2) && !MathLib::isFloat(value2))
|
||||
continue;
|
||||
|
||||
if (isSameExpression(comp1, comp2, _settings->library.functionpure))
|
||||
continue; // same expressions => only report that there are same expressions
|
||||
if (!isSameExpression(expr1, expr2, _settings->library.functionpure))
|
||||
continue;
|
||||
|
||||
const bool isfloat = astIsFloat(expr1, true) || MathLib::isFloat(value1) || astIsFloat(expr2, true) || MathLib::isFloat(value2);
|
||||
|
||||
// don't check floating point equality comparisons. that is bad
|
||||
// and deserves different warnings.
|
||||
if (isfloat && (op1 == "==" || op1 == "!=" || op2 == "==" || op2 == "!="))
|
||||
continue;
|
||||
|
||||
// evaluate if expression is always true/false
|
||||
bool alwaysTrue = true, alwaysFalse = true;
|
||||
bool firstTrue = true, secondTrue = true;
|
||||
for (int test = 1; test <= 5; ++test) {
|
||||
// test:
|
||||
// 1 => testvalue is less than both value1 and value2
|
||||
// 2 => testvalue is value1
|
||||
// 3 => testvalue is between value1 and value2
|
||||
// 4 => testvalue value2
|
||||
// 5 => testvalue is larger than both value1 and value2
|
||||
bool result1, result2;
|
||||
if (isfloat) {
|
||||
const double d1 = MathLib::toDoubleNumber(value1);
|
||||
const double d2 = MathLib::toDoubleNumber(value2);
|
||||
const double testvalue = getvalue<double>(test, d1, d2);
|
||||
result1 = checkFloatRelation(op1, testvalue, d1);
|
||||
result2 = checkFloatRelation(op2, testvalue, d2);
|
||||
} else {
|
||||
const MathLib::bigint i1 = MathLib::toLongNumber(value1);
|
||||
const MathLib::bigint i2 = MathLib::toLongNumber(value2);
|
||||
const MathLib::bigint testvalue = getvalue<MathLib::bigint>(test, i1, i2);
|
||||
result1 = checkIntRelation(op1, testvalue, i1);
|
||||
result2 = checkIntRelation(op2, testvalue, i2);
|
||||
}
|
||||
if (tok->str() == "&&") {
|
||||
alwaysTrue &= (result1 && result2);
|
||||
alwaysFalse &= !(result1 && result2);
|
||||
} else {
|
||||
alwaysTrue &= (result1 || result2);
|
||||
alwaysFalse &= !(result1 || result2);
|
||||
}
|
||||
firstTrue &= !(!result1 && result2);
|
||||
secondTrue &= !(result1 && !result2);
|
||||
}
|
||||
|
||||
const std::string cond1str = (expr1->isName() ? expr1->str() : "EXPR") + " " + op1 + " " + value1;
|
||||
const std::string cond2str = (expr2->isName() ? expr2->str() : "EXPR") + " " + op2 + " " + value2;
|
||||
if (warning && (alwaysTrue || alwaysFalse)) {
|
||||
const std::string text = cond1str + " " + tok->str() + " " + cond2str;
|
||||
incorrectLogicOperatorError(tok, text, alwaysTrue);
|
||||
} else if (style && secondTrue) {
|
||||
const std::string text = "If " + cond1str + ", the comparison " + cond2str +
|
||||
" is always " + (secondTrue ? "true" : "false") + ".";
|
||||
redundantConditionError(tok, text);
|
||||
} else if (style && firstTrue) {
|
||||
//const std::string text = "The comparison " + cond1str + " is always " +
|
||||
// (firstTrue ? "true" : "false") + " when " +
|
||||
// cond2str + ".";
|
||||
const std::string text = "If " + cond2str + ", the comparison " + cond1str +
|
||||
" is always " + (firstTrue ? "true" : "false") + ".";
|
||||
redundantConditionError(tok, text);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always)
|
||||
{
|
||||
if (always)
|
||||
reportError(tok, Severity::warning, "incorrectLogicOperator",
|
||||
"Logical disjunction always evaluates to true: " + condition + ".\n"
|
||||
"Logical disjunction always evaluates to true: " + condition + ". "
|
||||
"Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
else
|
||||
reportError(tok, Severity::warning, "incorrectLogicOperator",
|
||||
"Logical conjunction always evaluates to false: " + condition + ".\n"
|
||||
"Logical conjunction always evaluates to false: " + condition + ". "
|
||||
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
}
|
||||
|
||||
void CheckCondition::redundantConditionError(const Token *tok, const std::string &text)
|
||||
{
|
||||
reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text);
|
||||
}
|
||||
|
||||
//-----------------------------------------------------------------------------
|
||||
// Detect "(var % val1) > val2" where val2 is >= val1.
|
||||
//-----------------------------------------------------------------------------
|
||||
void CheckCondition::checkModuloAlwaysTrueFalse()
|
||||
{
|
||||
if (!_settings->isEnabled("warning"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (!tok->isComparisonOp())
|
||||
continue;
|
||||
const Token *num, *modulo;
|
||||
if (Token::simpleMatch(tok->astOperand1(), "%") && Token::Match(tok->astOperand2(), "%num%")) {
|
||||
modulo = tok->astOperand1();
|
||||
num = tok->astOperand2();
|
||||
} else if (Token::Match(tok->astOperand1(), "%num%") && Token::simpleMatch(tok->astOperand2(), "%")) {
|
||||
num = tok->astOperand1();
|
||||
modulo = tok->astOperand2();
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (Token::Match(modulo->astOperand2(), "%num%") &&
|
||||
MathLib::isLessEqual(modulo->astOperand2()->str(), num->str()))
|
||||
moduloAlwaysTrueFalseError(tok, modulo->astOperand2()->str());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal)
|
||||
{
|
||||
reportError(tok, Severity::warning, "moduloAlwaysTrueFalse",
|
||||
"Comparison of modulo result is predetermined, because it is always less than " + maxVal + ".");
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))'
|
||||
// Clarify condition '(a & b == c)' into '((a & b) == c)' or '(a & (b == c))'
|
||||
//---------------------------------------------------------------------------
|
||||
void CheckCondition::clarifyCondition()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
const bool isC = _tokenizer->isC();
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (Token::Match(tok, "( %var% [=&|^]")) {
|
||||
for (const Token *tok2 = tok->tokAt(3); tok2; tok2 = tok2->next()) {
|
||||
if (tok2->str() == "(" || tok2->str() == "[")
|
||||
tok2 = tok2->link();
|
||||
else if (tok2->type() == Token::eComparisonOp) {
|
||||
// This might be a template
|
||||
if (!isC && tok2->link())
|
||||
break;
|
||||
|
||||
clarifyConditionError(tok, tok->strAt(2) == "=", false);
|
||||
break;
|
||||
} else if (!tok2->isName() && !tok2->isNumber() && tok2->str() != ".")
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// using boolean result in bitwise operation ! x [&|^]
|
||||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (Token::Match(tok, "%comp%|!")) {
|
||||
if (tok->link()) // don't write false positives when templates are used
|
||||
continue;
|
||||
|
||||
const Token *tok2 = tok->next();
|
||||
|
||||
// Todo: There are false positives if '(' if encountered. It
|
||||
// is assumed there is something like '(char *)&..' and therefore
|
||||
// it bails out.
|
||||
if (Token::Match(tok2, "(|&"))
|
||||
continue;
|
||||
|
||||
while (tok2 && (tok2->isName() || tok2->isNumber() || Token::Match(tok2, ".|(|["))) {
|
||||
if (Token::Match(tok2, "(|["))
|
||||
tok2 = tok2->link();
|
||||
tok2 = tok2->next();
|
||||
}
|
||||
|
||||
if (Token::Match(tok2, "[&|^]")) {
|
||||
// don't write false positives when templates are used
|
||||
if (Token::Match(tok2, "&|* ,|>") || Token::simpleMatch(tok2->previous(), "const &"))
|
||||
continue;
|
||||
|
||||
// #3609 - CWinTraits<WS_CHILD|WS_VISIBLE>::..
|
||||
if (!isC && Token::Match(tok->previous(), "%var% <")) {
|
||||
const Token *tok3 = tok2;
|
||||
while (Token::Match(tok3, "[&|^] %var%"))
|
||||
tok3 = tok3->tokAt(2);
|
||||
if (Token::Match(tok3, ",|>"))
|
||||
continue;
|
||||
}
|
||||
|
||||
clarifyConditionError(tok, false, true);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckCondition::clarifyConditionError(const Token *tok, bool assign, bool boolop)
|
||||
{
|
||||
std::string errmsg;
|
||||
|
||||
if (assign)
|
||||
errmsg = "Suspicious condition (assignment + comparison); Clarify expression with parentheses.";
|
||||
|
||||
else if (boolop)
|
||||
errmsg = "Boolean result is used in bitwise operation. Clarify expression with parentheses.\n"
|
||||
"Suspicious expression. Boolean result is used in bitwise operation. The operator '!' "
|
||||
"and the comparison operators have higher precedence than bitwise operators. "
|
||||
"It is recommended that the expression is clarified with parentheses.";
|
||||
else
|
||||
errmsg = "Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n"
|
||||
"Suspicious condition. Comparison operators have higher precedence than bitwise operators. "
|
||||
"Please clarify the condition with parentheses.";
|
||||
|
||||
reportError(tok,
|
||||
Severity::style,
|
||||
"clarifyCondition",
|
||||
errmsg);
|
||||
}
|
|
@ -18,8 +18,8 @@
|
|||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
#ifndef checkassignifH
|
||||
#define checkassignifH
|
||||
#ifndef checkconditionH
|
||||
#define checkconditionH
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
#include "config.h"
|
||||
|
@ -30,30 +30,34 @@
|
|||
/// @{
|
||||
|
||||
/**
|
||||
* @brief Check for assignment / condition mismatches
|
||||
* @brief Check for condition mismatches
|
||||
*/
|
||||
|
||||
class CPPCHECKLIB CheckAssignIf : public Check {
|
||||
class CPPCHECKLIB CheckCondition : public Check {
|
||||
public:
|
||||
/** This constructor is used when registering the CheckAssignIf */
|
||||
CheckAssignIf() : Check(myName()) {
|
||||
CheckCondition() : Check(myName()) {
|
||||
}
|
||||
|
||||
/** This constructor is used when running checks. */
|
||||
CheckAssignIf(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
|
||||
CheckCondition(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
|
||||
: Check(myName(), tokenizer, settings, errorLogger) {
|
||||
}
|
||||
|
||||
void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
|
||||
CheckAssignIf checkAssignIf(tokenizer, settings, errorLogger);
|
||||
checkAssignIf.multiCondition();
|
||||
CheckCondition checkCondition(tokenizer, settings, errorLogger);
|
||||
checkCondition.multiCondition();
|
||||
checkCondition.clarifyCondition(); // not simplified because ifAssign
|
||||
}
|
||||
|
||||
/** @brief Run checks against the simplified token list */
|
||||
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
|
||||
CheckAssignIf checkAssignIf(tokenizer, settings, errorLogger);
|
||||
checkAssignIf.assignIf();
|
||||
checkAssignIf.comparison();
|
||||
CheckCondition checkCondition(tokenizer, settings, errorLogger);
|
||||
checkCondition.assignIf();
|
||||
checkCondition.comparison();
|
||||
checkCondition.oppositeInnerCondition();
|
||||
checkCondition.checkIncorrectLogicOperator();
|
||||
checkCondition.checkModuloAlwaysTrueFalse();
|
||||
}
|
||||
|
||||
/** mismatching assignment / comparison */
|
||||
|
@ -73,41 +77,69 @@ public:
|
|||
/** match 'if' and 'else if' conditions */
|
||||
void multiCondition();
|
||||
|
||||
/** To check the dead code in a program, which is inaccessible due to the counter-conditions check in nested-if statements **/
|
||||
void oppositeInnerCondition();
|
||||
|
||||
/** @brief %Check for testing for mutual exclusion over ||*/
|
||||
void checkIncorrectLogicOperator();
|
||||
|
||||
/** @brief %Check for suspicious usage of modulo (e.g. "if(var % 4 == 4)") */
|
||||
void checkModuloAlwaysTrueFalse();
|
||||
|
||||
/** @brief Suspicious condition (assignment+comparison) */
|
||||
void clarifyCondition();
|
||||
|
||||
private:
|
||||
|
||||
void assignIfError(const Token *tok1, const Token *tok2, const std::string &condition, bool result);
|
||||
|
||||
void mismatchingBitAndError(const Token *tok1, const MathLib::bigint num1, const Token *tok2, const MathLib::bigint num2);
|
||||
|
||||
void comparisonError(const Token *tok,
|
||||
const std::string &bitop,
|
||||
MathLib::bigint value1,
|
||||
const std::string &op,
|
||||
MathLib::bigint value2,
|
||||
bool result);
|
||||
|
||||
void multiConditionError(const Token *tok, unsigned int line1);
|
||||
|
||||
void oppositeInnerConditionError(const Token *tok1, const Token* tok2);
|
||||
|
||||
void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always);
|
||||
void redundantConditionError(const Token *tok, const std::string &text);
|
||||
|
||||
void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
|
||||
|
||||
void clarifyConditionError(const Token *tok, bool assign, bool boolop);
|
||||
|
||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
||||
CheckAssignIf c(0, settings, errorLogger);
|
||||
CheckCondition c(0, settings, errorLogger);
|
||||
|
||||
c.assignIfError(0, 0, "", false);
|
||||
c.comparisonError(0, "&", 6, "==", 1, false);
|
||||
c.multiConditionError(0,1);
|
||||
c.mismatchingBitAndError(0, 0xf0, 0, 1);
|
||||
c.oppositeInnerConditionError(0, 0);
|
||||
c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true);
|
||||
c.redundantConditionError(0, "If x > 11 the condition x > 10 is always true.");
|
||||
c.moduloAlwaysTrueFalseError(0, "1");
|
||||
c.clarifyConditionError(0, true, false);
|
||||
}
|
||||
|
||||
static std::string myName() {
|
||||
return "AssignIf";
|
||||
return "Condition";
|
||||
}
|
||||
|
||||
std::string classInfo() const {
|
||||
return "Match assignments and conditions:\n"
|
||||
return "Match conditions with assignments and other conditions:\n"
|
||||
"* Mismatching assignment and comparison => comparison is always true/false\n"
|
||||
"* Mismatching lhs and rhs in comparison => comparison is always true/false\n"
|
||||
"* Detect matching 'if' and 'else if' conditions\n"
|
||||
"* Mismatching bitand (a &= 0xf0; a &= 1; => a = 0)\n";
|
||||
"* Mismatching bitand (a &= 0xf0; a &= 1; => a = 0)\n"
|
||||
"* Find dead code which is inaccessible due to the counter-conditions check in nested if statements\n"
|
||||
"* condition that is always true/false\n"
|
||||
"* mutual exclusion over || always evaluating to true\n"
|
||||
"* Comparisons of modulo results that are always true/false.\n";
|
||||
}
|
||||
};
|
||||
/// @}
|
||||
//---------------------------------------------------------------------------
|
||||
#endif // checkassignifH
|
||||
#endif // checkconditionH
|
|
@ -32,7 +32,7 @@ namespace {
|
|||
CheckOther instance;
|
||||
}
|
||||
|
||||
static bool astIsFloat(const Token *tok, bool unknown)
|
||||
bool astIsFloat(const Token *tok, bool unknown)
|
||||
{
|
||||
if (tok->astOperand2() && (tok->str() == "." || tok->str() == "::"))
|
||||
return astIsFloat(tok->astOperand2(), unknown);
|
||||
|
@ -155,47 +155,6 @@ bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::
|
|||
return commuative_equals;
|
||||
}
|
||||
|
||||
static bool isOppositeCond(const Token * const cond1, const Token * const cond2, const std::set<std::string> &constFunctions)
|
||||
{
|
||||
if (!cond1 || !cond2)
|
||||
return false;
|
||||
|
||||
if (cond1->str() == "!")
|
||||
return isSameExpression(cond1->astOperand1(), cond2, constFunctions);
|
||||
|
||||
if (cond2->str() == "!")
|
||||
return isSameExpression(cond1, cond2->astOperand1(), constFunctions);
|
||||
|
||||
if (!cond1->isComparisonOp() || !cond2->isComparisonOp())
|
||||
return false;
|
||||
|
||||
const std::string &comp1 = cond1->str();
|
||||
|
||||
// condition found .. get comparator
|
||||
std::string comp2;
|
||||
if (isSameExpression(cond1->astOperand1(), cond2->astOperand1(), constFunctions) &&
|
||||
isSameExpression(cond1->astOperand2(), cond2->astOperand2(), constFunctions)) {
|
||||
comp2 = cond2->str();
|
||||
} else if (isSameExpression(cond1->astOperand1(), cond2->astOperand2(), constFunctions) &&
|
||||
isSameExpression(cond1->astOperand2(), cond2->astOperand1(), constFunctions)) {
|
||||
comp2 = cond2->str();
|
||||
if (comp2[0] == '>')
|
||||
comp2[0] = '<';
|
||||
else if (comp2[0] == '<')
|
||||
comp2[0] = '>';
|
||||
}
|
||||
|
||||
// is condition opposite?
|
||||
return ((comp1 == "==" && comp2 == "!=") ||
|
||||
(comp1 == "!=" && comp2 == "==") ||
|
||||
(comp1 == "<" && comp2 == ">=") ||
|
||||
(comp1 == "<" && comp2 == ">") ||
|
||||
(comp1 == "<=" && comp2 == ">") ||
|
||||
(comp1 == ">" && comp2 == "<=") ||
|
||||
(comp1 == ">" && comp2 == "<") ||
|
||||
(comp1 == ">=" && comp2 == "<"));
|
||||
}
|
||||
|
||||
//----------------------------------------------------------------------------------
|
||||
// The return value of fgetc(), getc(), ungetc(), getchar() etc. is an integer value.
|
||||
// If this return value is stored in a character variable and then compared
|
||||
|
@ -323,106 +282,6 @@ void CheckOther::clarifyCalculationError(const Token *tok, const std::string &op
|
|||
"The code '" + calc + "' should be written as either '" + s1 + "' or '" + s2 + "'.");
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Clarify condition '(x = a < 0)' into '((x = a) < 0)' or '(x = (a < 0))'
|
||||
// Clarify condition '(a & b == c)' into '((a & b) == c)' or '(a & (b == c))'
|
||||
//---------------------------------------------------------------------------
|
||||
void CheckOther::clarifyCondition()
|
||||
{
|
||||
if (!_settings->isEnabled("style"))
|
||||
return;
|
||||
|
||||
const bool isC = _tokenizer->isC();
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (Token::Match(tok, "( %var% [=&|^]")) {
|
||||
for (const Token *tok2 = tok->tokAt(3); tok2; tok2 = tok2->next()) {
|
||||
if (tok2->str() == "(" || tok2->str() == "[")
|
||||
tok2 = tok2->link();
|
||||
else if (tok2->type() == Token::eComparisonOp) {
|
||||
// This might be a template
|
||||
if (!isC && tok2->link())
|
||||
break;
|
||||
|
||||
clarifyConditionError(tok, tok->strAt(2) == "=", false);
|
||||
break;
|
||||
} else if (!tok2->isName() && !tok2->isNumber() && tok2->str() != ".")
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// using boolean result in bitwise operation ! x [&|^]
|
||||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (Token::Match(tok, "%comp%|!")) {
|
||||
if (tok->link()) // don't write false positives when templates are used
|
||||
continue;
|
||||
|
||||
const Token *tok2 = tok->next();
|
||||
|
||||
// Todo: There are false positives if '(' if encountered. It
|
||||
// is assumed there is something like '(char *)&..' and therefore
|
||||
// it bails out.
|
||||
if (Token::Match(tok2, "(|&"))
|
||||
continue;
|
||||
|
||||
while (tok2 && (tok2->isName() || tok2->isNumber() || Token::Match(tok2,".|(|["))) {
|
||||
if (Token::Match(tok2, "(|["))
|
||||
tok2 = tok2->link();
|
||||
tok2 = tok2->next();
|
||||
}
|
||||
|
||||
if (Token::Match(tok2, "[&|^]")) {
|
||||
// don't write false positives when templates are used
|
||||
if (Token::Match(tok2, "&|* ,|>") || Token::simpleMatch(tok2->previous(), "const &"))
|
||||
continue;
|
||||
|
||||
// #3609 - CWinTraits<WS_CHILD|WS_VISIBLE>::..
|
||||
if (!isC && Token::Match(tok->previous(), "%var% <")) {
|
||||
const Token *tok3 = tok2;
|
||||
while (Token::Match(tok3, "[&|^] %var%"))
|
||||
tok3 = tok3->tokAt(2);
|
||||
if (Token::Match(tok3, ",|>"))
|
||||
continue;
|
||||
}
|
||||
|
||||
clarifyConditionError(tok,false,true);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckOther::clarifyConditionError(const Token *tok, bool assign, bool boolop)
|
||||
{
|
||||
std::string errmsg;
|
||||
|
||||
if (assign)
|
||||
errmsg = "Suspicious condition (assignment + comparison); Clarify expression with parentheses.";
|
||||
|
||||
else if (boolop)
|
||||
errmsg = "Boolean result is used in bitwise operation. Clarify expression with parentheses.\n"
|
||||
"Suspicious expression. Boolean result is used in bitwise operation. The operator '!' "
|
||||
"and the comparison operators have higher precedence than bitwise operators. "
|
||||
"It is recommended that the expression is clarified with parentheses.";
|
||||
else
|
||||
errmsg = "Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n"
|
||||
"Suspicious condition. Comparison operators have higher precedence than bitwise operators. "
|
||||
"Please clarify the condition with parentheses.";
|
||||
|
||||
reportError(tok,
|
||||
Severity::style,
|
||||
"clarifyCondition",
|
||||
errmsg);
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Clarify (meaningless) statements like *foo++; with parentheses.
|
||||
//---------------------------------------------------------------------------
|
||||
|
@ -1178,249 +1037,6 @@ void CheckOther::suspiciousEqualityComparisonError(const Token* tok)
|
|||
}
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// if ((x != 1) || (x != 3)) // expression always true
|
||||
// if ((x == 1) && (x == 3)) // expression always false
|
||||
// if ((x < 1) && (x > 3)) // expression always false
|
||||
// if ((x > 3) || (x < 10)) // expression always true
|
||||
// if ((x > 5) && (x != 1)) // second comparison always true
|
||||
//
|
||||
// Check for suspect logic for an expression consisting of 2 comparison
|
||||
// expressions with a shared variable and constants and a logical operator
|
||||
// between them.
|
||||
//
|
||||
// Suggest a different logical operator when the logical operator between
|
||||
// the comparisons is probably wrong.
|
||||
//
|
||||
// Inform that second comparison is always true when first comparison is true.
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
static std::string invertOperatorForOperandSwap(std::string s)
|
||||
{
|
||||
if (s[0] == '<')
|
||||
s[0] = '>';
|
||||
else if (s[0] == '>')
|
||||
s[0] = '<';
|
||||
return s;
|
||||
}
|
||||
|
||||
static bool checkIntRelation(const std::string &op, const MathLib::bigint value1, const MathLib::bigint value2)
|
||||
{
|
||||
return (op == "==" && value1 == value2) ||
|
||||
(op == "!=" && value1 != value2) ||
|
||||
(op == ">" && value1 > value2) ||
|
||||
(op == ">=" && value1 >= value2) ||
|
||||
(op == "<" && value1 < value2) ||
|
||||
(op == "<=" && value1 <= value2);
|
||||
}
|
||||
|
||||
static bool checkFloatRelation(const std::string &op, const double value1, const double value2)
|
||||
{
|
||||
return (op == ">" && value1 > value2) ||
|
||||
(op == ">=" && value1 >= value2) ||
|
||||
(op == "<" && value1 < value2) ||
|
||||
(op == "<=" && value1 <= value2);
|
||||
}
|
||||
|
||||
template<class T> static T getvalue(const int test, const T value1, const T value2)
|
||||
{
|
||||
// test:
|
||||
// 1 => return value that is less than both value1 and value2
|
||||
// 2 => return value1
|
||||
// 3 => return value that is between value1 and value2
|
||||
// 4 => return value2
|
||||
// 5 => return value that is larger than both value1 and value2
|
||||
switch (test) {
|
||||
case 1: {
|
||||
T ret = std::min(value1, value2);
|
||||
if ((ret - (T)1) < ret)
|
||||
return ret - (T)1;
|
||||
else if ((ret / (T)2) < ret)
|
||||
return ret / (T)2;
|
||||
else if ((ret * (T)2) < ret)
|
||||
return ret * (T)2;
|
||||
return ret;
|
||||
}
|
||||
case 2:
|
||||
return value1;
|
||||
case 3:
|
||||
return (value1 + value2) / (T)2;
|
||||
case 4:
|
||||
return value2;
|
||||
case 5: {
|
||||
T ret = std::max(value1, value2);
|
||||
if ((ret + (T)1) > ret)
|
||||
return ret + (T)1;
|
||||
else if ((ret / (T)2) > ret)
|
||||
return ret / (T)2;
|
||||
else if ((ret * (T)2) > ret)
|
||||
return ret * (T)2;
|
||||
return ret;
|
||||
}
|
||||
};
|
||||
return 0;
|
||||
}
|
||||
|
||||
void CheckOther::checkIncorrectLogicOperator()
|
||||
{
|
||||
bool style = _settings->isEnabled("style");
|
||||
bool warning = _settings->isEnabled("warning");
|
||||
if (!style && !warning)
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t ii = 0; ii < functions; ++ii) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[ii];
|
||||
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
// Opposite comparisons
|
||||
if (Token::Match(tok, "%oror%|&&") &&
|
||||
tok->astOperand1() &&
|
||||
tok->astOperand2() &&
|
||||
(tok->astOperand1()->isName() || tok->astOperand2()->isName()) &&
|
||||
isOppositeCond(tok->astOperand1(), tok->astOperand2(), _settings->library.functionpure)) {
|
||||
|
||||
const bool alwaysTrue(tok->str() == "||");
|
||||
incorrectLogicOperatorError(tok, tok->expressionString(), alwaysTrue);
|
||||
}
|
||||
|
||||
else if (Token::Match(tok, "&&|%oror%")) {
|
||||
// Comparison #1 (LHS)
|
||||
const Token *comp1 = tok->astOperand1();
|
||||
if (comp1 && comp1->str() == tok->str())
|
||||
comp1 = comp1->astOperand2();
|
||||
|
||||
// Comparison #2 (RHS)
|
||||
const Token *comp2 = tok->astOperand2();
|
||||
|
||||
if (!comp1 || !comp1->isComparisonOp() || !comp1->astOperand1() || !comp1->astOperand2())
|
||||
continue;
|
||||
if (!comp2 || !comp2->isComparisonOp() || !comp2->astOperand1() || !comp2->astOperand2())
|
||||
continue;
|
||||
|
||||
std::string op1, value1;
|
||||
const Token *expr1;
|
||||
if (comp1->astOperand1()->isLiteral()) {
|
||||
op1 = invertOperatorForOperandSwap(comp1->str());
|
||||
value1 = comp1->astOperand1()->str();
|
||||
expr1 = comp1->astOperand2();
|
||||
} else if (comp1->astOperand2()->isLiteral()) {
|
||||
op1 = comp1->str();
|
||||
value1 = comp1->astOperand2()->str();
|
||||
expr1 = comp1->astOperand1();
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
std::string op2, value2;
|
||||
const Token *expr2;
|
||||
if (comp2->astOperand1()->isLiteral()) {
|
||||
op2 = invertOperatorForOperandSwap(comp2->str());
|
||||
value2 = comp2->astOperand1()->str();
|
||||
expr2 = comp2->astOperand2();
|
||||
} else if (comp2->astOperand2()->isLiteral()) {
|
||||
op2 = comp2->str();
|
||||
value2 = comp2->astOperand2()->str();
|
||||
expr2 = comp2->astOperand1();
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only float and int values are currently handled
|
||||
if (!MathLib::isInt(value1) && !MathLib::isFloat(value1))
|
||||
continue;
|
||||
if (!MathLib::isInt(value2) && !MathLib::isFloat(value2))
|
||||
continue;
|
||||
|
||||
if (isSameExpression(comp1, comp2, _settings->library.functionpure))
|
||||
continue; // same expressions => only report that there are same expressions
|
||||
if (!isSameExpression(expr1, expr2, _settings->library.functionpure))
|
||||
continue;
|
||||
|
||||
const bool isfloat = astIsFloat(expr1,true) || MathLib::isFloat(value1) || astIsFloat(expr2,true) || MathLib::isFloat(value2);
|
||||
|
||||
// don't check floating point equality comparisons. that is bad
|
||||
// and deserves different warnings.
|
||||
if (isfloat && (op1=="==" || op1=="!=" || op2=="==" || op2=="!="))
|
||||
continue;
|
||||
|
||||
// evaluate if expression is always true/false
|
||||
bool alwaysTrue = true, alwaysFalse = true;
|
||||
bool firstTrue = true, secondTrue = true;
|
||||
for (int test = 1; test <= 5; ++test) {
|
||||
// test:
|
||||
// 1 => testvalue is less than both value1 and value2
|
||||
// 2 => testvalue is value1
|
||||
// 3 => testvalue is between value1 and value2
|
||||
// 4 => testvalue value2
|
||||
// 5 => testvalue is larger than both value1 and value2
|
||||
bool result1, result2;
|
||||
if (isfloat) {
|
||||
const double d1 = MathLib::toDoubleNumber(value1);
|
||||
const double d2 = MathLib::toDoubleNumber(value2);
|
||||
const double testvalue = getvalue<double>(test, d1, d2);
|
||||
result1 = checkFloatRelation(op1, testvalue, d1);
|
||||
result2 = checkFloatRelation(op2, testvalue, d2);
|
||||
} else {
|
||||
const MathLib::bigint i1 = MathLib::toLongNumber(value1);
|
||||
const MathLib::bigint i2 = MathLib::toLongNumber(value2);
|
||||
const MathLib::bigint testvalue = getvalue<MathLib::bigint>(test, i1, i2);
|
||||
result1 = checkIntRelation(op1, testvalue, i1);
|
||||
result2 = checkIntRelation(op2, testvalue, i2);
|
||||
}
|
||||
if (tok->str() == "&&") {
|
||||
alwaysTrue &= (result1 && result2);
|
||||
alwaysFalse &= !(result1 && result2);
|
||||
} else {
|
||||
alwaysTrue &= (result1 || result2);
|
||||
alwaysFalse &= !(result1 || result2);
|
||||
}
|
||||
firstTrue &= !(!result1 && result2);
|
||||
secondTrue &= !(result1 && !result2);
|
||||
}
|
||||
|
||||
const std::string cond1str = (expr1->isName() ? expr1->str() : "EXPR") + " " + op1 + " " + value1;
|
||||
const std::string cond2str = (expr2->isName() ? expr2->str() : "EXPR") + " " + op2 + " " + value2;
|
||||
if (warning && (alwaysTrue || alwaysFalse)) {
|
||||
const std::string text = cond1str + " " + tok->str() + " " + cond2str;
|
||||
incorrectLogicOperatorError(tok, text, alwaysTrue);
|
||||
} else if (style && secondTrue) {
|
||||
const std::string text = "If " + cond1str + ", the comparison " + cond2str +
|
||||
" is always " + (secondTrue ? "true" : "false") + ".";
|
||||
redundantConditionError(tok, text);
|
||||
} else if (style && firstTrue) {
|
||||
//const std::string text = "The comparison " + cond1str + " is always " +
|
||||
// (firstTrue ? "true" : "false") + " when " +
|
||||
// cond2str + ".";
|
||||
const std::string text = "If " + cond2str + ", the comparison " + cond1str +
|
||||
" is always " + (firstTrue ? "true" : "false") + ".";
|
||||
redundantConditionError(tok, text);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckOther::incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always)
|
||||
{
|
||||
if (always)
|
||||
reportError(tok, Severity::warning, "incorrectLogicOperator",
|
||||
"Logical disjunction always evaluates to true: " + condition + ".\n"
|
||||
"Logical disjunction always evaluates to true: " + condition + ". "
|
||||
"Are these conditions necessary? Did you intend to use && instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
else
|
||||
reportError(tok, Severity::warning, "incorrectLogicOperator",
|
||||
"Logical conjunction always evaluates to false: " + condition + ".\n"
|
||||
"Logical conjunction always evaluates to false: " + condition + ". "
|
||||
"Are these conditions necessary? Did you intend to use || instead? Are the numbers correct? Are you comparing the correct variables?");
|
||||
}
|
||||
|
||||
void CheckOther::redundantConditionError(const Token *tok, const std::string &text)
|
||||
{
|
||||
reportError(tok, Severity::style, "redundantCondition", "Redundant condition: " + text);
|
||||
}
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// strtol(str, 0, radix) <- radix must be 0 or 2-36
|
||||
//---------------------------------------------------------------------------
|
||||
|
@ -2772,45 +2388,6 @@ void CheckOther::checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* to
|
|||
"for both parameters leads to a statement which is always " + strResult + ".");
|
||||
}
|
||||
|
||||
//-----------------------------------------------------------------------------
|
||||
// Detect "(var % val1) > val2" where val2 is >= val1.
|
||||
//-----------------------------------------------------------------------------
|
||||
void CheckOther::checkModuloAlwaysTrueFalse()
|
||||
{
|
||||
if (!_settings->isEnabled("warning"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
const std::size_t functions = symbolDatabase->functionScopes.size();
|
||||
for (std::size_t i = 0; i < functions; ++i) {
|
||||
const Scope * scope = symbolDatabase->functionScopes[i];
|
||||
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
|
||||
if (!tok->isComparisonOp())
|
||||
continue;
|
||||
const Token *num,*modulo;
|
||||
if (Token::simpleMatch(tok->astOperand1(), "%") && Token::Match(tok->astOperand2(), "%num%")) {
|
||||
modulo = tok->astOperand1();
|
||||
num = tok->astOperand2();
|
||||
} else if (Token::Match(tok->astOperand1(), "%num%") && Token::simpleMatch(tok->astOperand2(), "%")) {
|
||||
num = tok->astOperand1();
|
||||
modulo = tok->astOperand2();
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (Token::Match(modulo->astOperand2(), "%num%") &&
|
||||
MathLib::isLessEqual(modulo->astOperand2()->str(),num->str()))
|
||||
moduloAlwaysTrueFalseError(tok, modulo->astOperand2()->str());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void CheckOther::moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal)
|
||||
{
|
||||
reportError(tok, Severity::warning, "moduloAlwaysTrueFalse",
|
||||
"Comparison of modulo result is predetermined, because it is always less than " + maxVal + ".");
|
||||
}
|
||||
|
||||
//-----------------------------------------------------------------------------
|
||||
// Check for code like:
|
||||
// seteuid(geteuid()) or setuid(getuid()), which first gets and then sets the
|
||||
|
@ -3097,107 +2674,6 @@ void CheckOther::incompleteArrayFillError(const Token* tok, const std::string& b
|
|||
"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);
|
||||
}
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Detect oppositing inner and outer conditions
|
||||
//---------------------------------------------------------------------------
|
||||
|
||||
void CheckOther::oppositeInnerCondition()
|
||||
{
|
||||
if (!_settings->isEnabled("warning"))
|
||||
return;
|
||||
|
||||
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||
|
||||
for (std::list<Scope>::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
|
||||
if (scope->type != Scope::eIf)
|
||||
continue;
|
||||
|
||||
if (!Token::simpleMatch(scope->classDef->linkAt(1), ") {"))
|
||||
continue;
|
||||
|
||||
bool nonlocal = false; // nonlocal variable used in condition
|
||||
std::set<unsigned int> vars; // variables used in condition
|
||||
for (const Token *cond = scope->classDef; cond != scope->classDef->linkAt(1); cond = cond->next()) {
|
||||
if (cond->varId()) {
|
||||
vars.insert(cond->varId());
|
||||
const Variable *var = cond->variable();
|
||||
nonlocal |= (var && (!var->isLocal() || var->isStatic()) && !var->isArgument());
|
||||
// TODO: if var is pointer check what it points at
|
||||
nonlocal |= (var && (var->isPointer() || var->isReference()));
|
||||
} else if (cond->isName()) {
|
||||
// varid is 0. this is possibly a nonlocal variable..
|
||||
nonlocal |= (cond->astParent() && cond->astParent()->isConstOp());
|
||||
}
|
||||
}
|
||||
|
||||
// parse until inner condition is reached..
|
||||
const Token *ifToken = nullptr;
|
||||
for (const Token *tok = scope->classStart; tok && tok != scope->classEnd; tok = tok->next()) {
|
||||
if (Token::simpleMatch(tok, "if (")) {
|
||||
ifToken = tok;
|
||||
break;
|
||||
}
|
||||
if (Token::Match(tok,"%type% (") && nonlocal) // function call -> bailout if there are nonlocal variables
|
||||
break;
|
||||
else if ((tok->varId() && vars.find(tok->varId()) != vars.end()) ||
|
||||
(!tok->varId() && nonlocal)) {
|
||||
if (Token::Match(tok, "%var% ++|--|="))
|
||||
break;
|
||||
if (Token::Match(tok,"%var% [")) {
|
||||
const Token *tok2 = tok->linkAt(1);
|
||||
while (Token::simpleMatch(tok2, "] ["))
|
||||
tok2 = tok2->linkAt(1);
|
||||
if (Token::simpleMatch(tok2, "] ="))
|
||||
break;
|
||||
}
|
||||
if (Token::Match(tok->previous(), "++|--|& %var%"))
|
||||
break;
|
||||
if (Token::Match(tok->previous(), "[(,] %var% [,)]")) {
|
||||
// is variable unchanged? default is false..
|
||||
bool unchanged = false;
|
||||
|
||||
// locate start parentheses in function call..
|
||||
unsigned int argumentNumber = 0;
|
||||
const Token *start = tok->previous();
|
||||
while (start && start->str() == ",") {
|
||||
start = start->astParent();
|
||||
++argumentNumber;
|
||||
}
|
||||
|
||||
start = start ? start->previous() : nullptr;
|
||||
if (start && start->function()) {
|
||||
const Variable *arg = start->function()->getArgumentVar(argumentNumber);
|
||||
if (arg && !arg->isPointer() && !arg->isReference())
|
||||
unchanged = true;
|
||||
}
|
||||
|
||||
if (!unchanged)
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!ifToken)
|
||||
continue;
|
||||
|
||||
// Condition..
|
||||
const Token *cond1 = scope->classDef->next()->astOperand2();
|
||||
const Token *cond2 = ifToken->next()->astOperand2();
|
||||
|
||||
if (isOppositeCond(cond1, cond2, _settings->library.functionpure))
|
||||
oppositeInnerConditionError(scope->classDef, cond2);
|
||||
}
|
||||
}
|
||||
|
||||
void CheckOther::oppositeInnerConditionError(const Token *tok1, const Token* tok2)
|
||||
{
|
||||
std::list<const Token*> callstack;
|
||||
callstack.push_back(tok1);
|
||||
callstack.push_back(tok2);
|
||||
reportError(callstack, Severity::warning, "oppositeInnerCondition", "Opposite conditions in nested 'if' blocks lead to a dead code block.");
|
||||
}
|
||||
|
||||
|
||||
//---------------------------------------------------------------------------
|
||||
// Detect NULL being passed to variadic function.
|
||||
//---------------------------------------------------------------------------
|
||||
|
|
|
@ -31,6 +31,9 @@ class Variable;
|
|||
/** Is expressions same? */
|
||||
bool isSameExpression(const Token *tok1, const Token *tok2, const std::set<std::string> &constFunctions);
|
||||
|
||||
/** Is expression of floating point type? */
|
||||
bool astIsFloat(const Token *tok, bool unknown);
|
||||
|
||||
|
||||
/// @addtogroup Checks
|
||||
/// @{
|
||||
|
@ -66,7 +69,6 @@ public:
|
|||
checkOther.checkUnreachableCode();
|
||||
checkOther.checkSuspiciousSemicolon();
|
||||
checkOther.checkVariableScope();
|
||||
checkOther.clarifyCondition(); // not simplified because ifAssign
|
||||
checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574)
|
||||
checkOther.checkIncompleteArrayFill();
|
||||
checkOther.checkVarFuncNullUB();
|
||||
|
@ -79,7 +81,6 @@ public:
|
|||
CheckOther checkOther(tokenizer, settings, errorLogger);
|
||||
|
||||
// Checks
|
||||
checkOther.oppositeInnerCondition();
|
||||
checkOther.clarifyCalculation();
|
||||
checkOther.clarifyStatement();
|
||||
checkOther.checkConstantFunctionParameter();
|
||||
|
@ -91,12 +92,10 @@ public:
|
|||
checkOther.checkMathFunctions();
|
||||
|
||||
checkOther.redundantGetAndSetUserId();
|
||||
checkOther.checkIncorrectLogicOperator();
|
||||
checkOther.checkMisusedScopedObject();
|
||||
checkOther.checkMemsetZeroBytes();
|
||||
checkOther.checkMemsetInvalid2ndParam();
|
||||
checkOther.checkSwitchCaseFallThrough();
|
||||
checkOther.checkModuloAlwaysTrueFalse();
|
||||
checkOther.checkPipeParameterSize();
|
||||
|
||||
checkOther.checkInvalidFree();
|
||||
|
@ -107,15 +106,9 @@ public:
|
|||
checkOther.checkComparisonFunctionIsAlwaysTrueOrFalse();
|
||||
}
|
||||
|
||||
/** To check the dead code in a program, which is inaccessible due to the counter-conditions check in nested-if statements **/
|
||||
void oppositeInnerCondition();
|
||||
|
||||
/** @brief Clarify calculation for ".. a * b ? .." */
|
||||
void clarifyCalculation();
|
||||
|
||||
/** @brief Suspicious condition (assignment+comparison) */
|
||||
void clarifyCondition();
|
||||
|
||||
/** @brief Suspicious statement like '*A++;' */
|
||||
void clarifyStatement();
|
||||
|
||||
|
@ -184,9 +177,6 @@ public:
|
|||
/** @brief %Check for switch case fall through without comment */
|
||||
void checkSwitchCaseFallThrough();
|
||||
|
||||
/** @brief %Check for testing for mutual exclusion over ||*/
|
||||
void checkIncorrectLogicOperator();
|
||||
|
||||
/** @brief %Check for objects that are destroyed immediately */
|
||||
void checkMisusedScopedObject();
|
||||
|
||||
|
@ -205,9 +195,6 @@ public:
|
|||
/** @brief %Check for suspicious code with the same expression on both sides of operator (e.g "if (a && a)") */
|
||||
void checkDuplicateExpression();
|
||||
|
||||
/** @brief %Check for suspicious usage of modulo (e.g. "if(var % 4 == 4)") */
|
||||
void checkModuloAlwaysTrueFalse();
|
||||
|
||||
/** @brief %Check for code that gets never executed, such as duplicate break statements */
|
||||
void checkUnreachableCode();
|
||||
|
||||
|
@ -254,9 +241,7 @@ private:
|
|||
void checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* tok, const std::string &strFunctionName, const std::string &varName, const bool result);
|
||||
void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName);
|
||||
void checkPipeParameterSizeError(const Token *tok, const std::string &strVarName, const std::string &strDim);
|
||||
void oppositeInnerConditionError(const Token *tok1, const Token* tok2);
|
||||
void clarifyCalculationError(const Token *tok, const std::string &op);
|
||||
void clarifyConditionError(const Token *tok, bool assign, bool boolop);
|
||||
void clarifyStatementError(const Token* tok);
|
||||
void redundantGetAndSetUserIdError(const Token *tok);
|
||||
void cstyleCastError(const Token *tok);
|
||||
|
@ -282,8 +267,6 @@ private:
|
|||
void suspiciousCaseInSwitchError(const Token* tok, const std::string& operatorString);
|
||||
void suspiciousEqualityComparisonError(const Token* tok);
|
||||
void selfAssignmentError(const Token *tok, const std::string &varname);
|
||||
void incorrectLogicOperatorError(const Token *tok, const std::string &condition, bool always);
|
||||
void redundantConditionError(const Token *tok, const std::string &text);
|
||||
void misusedScopeObjectError(const Token *tok, const std::string &varname);
|
||||
void memsetZeroBytesError(const Token *tok, const std::string &varname);
|
||||
void memsetFloatError(const Token *tok, const std::string &var_value);
|
||||
|
@ -301,7 +284,6 @@ private:
|
|||
void pointerPositiveError(const Token *tok, bool inconclusive);
|
||||
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);
|
||||
void redundantCopyError(const Token *tok, const std::string &varname);
|
||||
void incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean);
|
||||
|
@ -332,7 +314,6 @@ private:
|
|||
// style/warning
|
||||
c.checkComparisonFunctionIsAlwaysTrueOrFalseError(0,"isless","varName",false);
|
||||
c.checkCastIntToCharAndBackError(0,"func_name");
|
||||
c.oppositeInnerConditionError(0, 0);
|
||||
c.cstyleCastError(0);
|
||||
c.passedByValueError(0, "parametername");
|
||||
c.constStatementError(0, "type");
|
||||
|
@ -345,13 +326,10 @@ private:
|
|||
c.suspiciousCaseInSwitchError(0, "||");
|
||||
c.suspiciousEqualityComparisonError(0);
|
||||
c.selfAssignmentError(0, "varname");
|
||||
c.incorrectLogicOperatorError(0, "foo > 3 && foo < 4", true);
|
||||
c.redundantConditionError(0, "If x > 11 the condition x > 10 is always true.");
|
||||
c.memsetZeroBytesError(0, "varname");
|
||||
c.memsetFloatError(0, "varname");
|
||||
c.memsetValueOutOfRangeError(0, "varname");
|
||||
c.clarifyCalculationError(0, "+");
|
||||
c.clarifyConditionError(0, true, false);
|
||||
c.clarifyStatementError(0);
|
||||
c.duplicateBranchError(0, 0);
|
||||
c.duplicateExpressionError(0, 0, "&&");
|
||||
|
@ -362,7 +340,6 @@ private:
|
|||
c.pointerLessThanZeroError(0, false);
|
||||
c.pointerPositiveError(0, false);
|
||||
c.SuspiciousSemicolonError(0);
|
||||
c.moduloAlwaysTrueFalseError(0, "1");
|
||||
c.incompleteArrayFillError(0, "buffer", "memset", false);
|
||||
c.varFuncNullUBError(0);
|
||||
c.nanInArithmeticExpressionError(0);
|
||||
|
@ -400,7 +377,6 @@ private:
|
|||
"* memset() with a float as the 2nd parameter\n"
|
||||
|
||||
// style
|
||||
"* Find dead code which is inaccessible due to the counter-conditions check in nested if statements\n"
|
||||
"* C-style pointer cast in cpp file\n"
|
||||
"* casting between incompatible pointer types\n"
|
||||
"* redundant if\n"
|
||||
|
@ -409,7 +385,6 @@ private:
|
|||
"* [[IncompleteStatement|Incomplete statement]]\n"
|
||||
"* [[charvar|check how signed char variables are used]]\n"
|
||||
"* variable scope can be limited\n"
|
||||
"* condition that is always true/false\n"
|
||||
"* unusual pointer arithmetic. For example: \"abc\" + 'd'\n"
|
||||
"* redundant assignment in a switch statement\n"
|
||||
"* redundant pre/post operation in a switch statement\n"
|
||||
|
@ -418,17 +393,14 @@ private:
|
|||
"* assignment of a variable to itself\n"
|
||||
"* Suspicious case labels in switch()\n"
|
||||
"* Suspicious equality comparisons\n"
|
||||
"* mutual exclusion over || always evaluating to true\n"
|
||||
"* Comparison of values leading always to true or false\n"
|
||||
"* Clarify calculation with parentheses\n"
|
||||
"* suspicious condition (assignment+comparison)\n"
|
||||
"* suspicious comparison of '\\0' with a char* variable\n"
|
||||
"* duplicate break statement\n"
|
||||
"* unreachable code\n"
|
||||
"* testing if unsigned variable is negative\n"
|
||||
"* testing is unsigned variable is positive\n"
|
||||
"* Suspicious use of ; at the end of 'if/for/while' statement.\n"
|
||||
"* Comparisons of modulo results that are always true/false.\n"
|
||||
"* Array filled incompletely using memset/memcpy/memmove.\n"
|
||||
"* redundant get and set function of user id (--std=posix).\n"
|
||||
"* Passing NULL pointer to function with variable number of arguments leads to UB on some platforms.\n"
|
||||
|
|
|
@ -39,12 +39,12 @@
|
|||
<ClCompile Include="check.cpp" />
|
||||
<ClCompile Include="check64bit.cpp" />
|
||||
<ClCompile Include="checkassert.cpp" />
|
||||
<ClCompile Include="checkassignif.cpp" />
|
||||
<ClCompile Include="checkautovariables.cpp" />
|
||||
<ClCompile Include="checkbool.cpp" />
|
||||
<ClCompile Include="checkboost.cpp" />
|
||||
<ClCompile Include="checkbufferoverrun.cpp" />
|
||||
<ClCompile Include="checkclass.cpp" />
|
||||
<ClCompile Include="checkcondition.cpp" />
|
||||
<ClCompile Include="checkstring.cpp" />
|
||||
<ClCompile Include="checkexceptionsafety.cpp" />
|
||||
<ClCompile Include="checkinternal.cpp" />
|
||||
|
@ -84,12 +84,12 @@
|
|||
<ClInclude Include="check.h" />
|
||||
<ClInclude Include="check64bit.h" />
|
||||
<ClInclude Include="checkassert.h" />
|
||||
<ClInclude Include="checkassignif.h" />
|
||||
<ClInclude Include="checkautovariables.h" />
|
||||
<ClInclude Include="checkbool.h" />
|
||||
<ClInclude Include="checkboost.h" />
|
||||
<ClInclude Include="checkbufferoverrun.h" />
|
||||
<ClInclude Include="checkclass.h" />
|
||||
<ClInclude Include="checkcondition.h" />
|
||||
<ClInclude Include="checkstring.h" />
|
||||
<ClInclude Include="checkexceptionsafety.h" />
|
||||
<ClInclude Include="checkinternal.h" />
|
||||
|
|
|
@ -95,9 +95,6 @@
|
|||
<ClCompile Include="checkboost.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="checkassignif.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="checkinternal.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
|
@ -140,6 +137,9 @@
|
|||
<ClCompile Include="checkstring.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="checkcondition.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<ClInclude Include="checkbufferoverrun.h">
|
||||
|
@ -226,9 +226,6 @@
|
|||
<ClInclude Include="check64bit.h">
|
||||
<Filter>Header Files</Filter>
|
||||
</ClInclude>
|
||||
<ClInclude Include="checkassignif.h">
|
||||
<Filter>Header Files</Filter>
|
||||
</ClInclude>
|
||||
<ClInclude Include="checkautovariables.h">
|
||||
<Filter>Header Files</Filter>
|
||||
</ClInclude>
|
||||
|
@ -277,6 +274,9 @@
|
|||
<ClInclude Include="checkstring.h">
|
||||
<Filter>Header Files</Filter>
|
||||
</ClInclude>
|
||||
<ClInclude Include="checkcondition.h">
|
||||
<Filter>Header Files</Filter>
|
||||
</ClInclude>
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<ResourceCompile Include="version.rc" />
|
||||
|
|
|
@ -1,395 +0,0 @@
|
|||
/*
|
||||
* Cppcheck - A tool for static C/C++ code analysis
|
||||
* Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
|
||||
#include "tokenize.h"
|
||||
#include "checkassignif.h"
|
||||
#include "testsuite.h"
|
||||
#include <sstream>
|
||||
|
||||
extern std::ostringstream errout;
|
||||
|
||||
class TestAssignIf : public TestFixture {
|
||||
public:
|
||||
TestAssignIf() : TestFixture("TestAssignIf") {
|
||||
}
|
||||
|
||||
private:
|
||||
|
||||
|
||||
void run() {
|
||||
TEST_CASE(assignAndCompare); // assignment and comparison don't match
|
||||
TEST_CASE(mismatchingBitAnd); // overlapping bitmasks
|
||||
TEST_CASE(compare); // mismatching LHS/RHS in comparison
|
||||
TEST_CASE(multicompare); // mismatching comparisons
|
||||
TEST_CASE(duplicateIf); // duplicate conditions in if and else-if
|
||||
TEST_CASE(invalidMissingSemicolon); // crash as of #5867
|
||||
}
|
||||
|
||||
void check(const char code[], bool validate=true) {
|
||||
// Clear the error buffer..
|
||||
errout.str("");
|
||||
|
||||
Settings settings;
|
||||
settings.addEnabled("style");
|
||||
|
||||
CheckAssignIf checkAssignIf;
|
||||
|
||||
// Tokenize..
|
||||
Tokenizer tokenizer(&settings, this);
|
||||
std::istringstream istr(code);
|
||||
tokenizer.tokenize(istr, "test.cpp");
|
||||
checkAssignIf.runChecks(&tokenizer, &settings, this);
|
||||
const std::string str1(tokenizer.tokens()->stringifyList(0,true));
|
||||
tokenizer.simplifyTokenList2();
|
||||
const std::string str2(tokenizer.tokens()->stringifyList(0,true));
|
||||
checkAssignIf.runSimplifiedChecks(&tokenizer, &settings, this);
|
||||
|
||||
// Ensure that the test case is not bad.
|
||||
if (validate && str1 != str2) {
|
||||
warn(("Unsimplified code in test case. It looks like this test "
|
||||
"should either be cleaned up or moved to TestTokenizer or "
|
||||
"TestSimplifyTokens instead.\nstr1="+str1+"\nstr2="+str2).c_str());
|
||||
}
|
||||
}
|
||||
|
||||
void assignAndCompare() {
|
||||
// &
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" int y = x & 4;\n"
|
||||
" if (y == 3);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
|
||||
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" int y = x & 4;\n"
|
||||
" if (y != 3);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y!=3' is always true.\n", errout.str());
|
||||
|
||||
// |
|
||||
check("void foo(int x) {\n"
|
||||
" int y = x | 0x14;\n"
|
||||
" if (y == 0x710);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==1808' is always false.\n", errout.str());
|
||||
|
||||
check("void foo(int x) {\n"
|
||||
" int y = x | 0x14;\n"
|
||||
" if (y == 0x71f);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// various simple assignments
|
||||
check("void foo(int x) {\n"
|
||||
" int y = (x+1) | 1;\n"
|
||||
" if (y == 2);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==2' is always false.\n", errout.str());
|
||||
|
||||
check("void foo() {\n"
|
||||
" int y = 1 | x();\n"
|
||||
" if (y == 2);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==2' is always false.\n", errout.str());
|
||||
|
||||
// multiple conditions
|
||||
check("void foo(int x) {\n"
|
||||
" int y = x & 4;\n"
|
||||
" if ((y == 3) && (z == 1));\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
|
||||
|
||||
check("void foo(int x) {\n"
|
||||
" int y = x & 4;\n"
|
||||
" if ((x==123) || ((y == 3) && (z == 1)));\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==3' is always false.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" int y = x & 7;\n"
|
||||
" if (setvalue(&y) && y != 8);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// recursive checking into scopes
|
||||
check("void f(int x) {\n"
|
||||
" int y = x & 7;\n"
|
||||
" if (z) y=0;\n"
|
||||
" else { if (y==8); }\n" // always false
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y==8' is always false.\n", errout.str());
|
||||
|
||||
// while
|
||||
check("void f(int x) {\n"
|
||||
" int y = x & 7;\n"
|
||||
" while (y==8);\n" // local variable => always false
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching assignment and comparison, comparison 'y==8' is always false.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" extern int y; y = x & 7;\n"
|
||||
" while (y==8);\n" // non-local variable => no error
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// calling function
|
||||
check("void f(int x) {\n"
|
||||
" int y = x & 7;\n"
|
||||
" do_something();\n"
|
||||
" if (y==8);\n" // local variable => always false
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Mismatching assignment and comparison, comparison 'y==8' is always false.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" int y = x & 7;\n"
|
||||
" do_something(&y);\n" // passing variable => no error
|
||||
" if (y==8);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void do_something(int);\n"
|
||||
"void f(int x) {\n"
|
||||
" int y = x & 7;\n"
|
||||
" do_something(y);\n"
|
||||
" if (y==8);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (style) Mismatching assignment and comparison, comparison 'y==8' is always false.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" extern int y; y = x & 7;\n"
|
||||
" do_something();\n"
|
||||
" if (y==8);\n" // non-local variable => no error
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #4434 : false positive: ?:
|
||||
check("void f(int x) {\n"
|
||||
" x = x & 1;\n"
|
||||
" x = x & 1 ? 1 : -1;\n"
|
||||
" if(x != -1) { }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #4735
|
||||
check("void f() {\n"
|
||||
" int x = *(char*)&0x12345678;\n"
|
||||
" if (x==18) { }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// bailout: no variable info
|
||||
check("void foo(int x) {\n"
|
||||
" y = 2 | x;\n" // y not declared => no error
|
||||
" if(y == 1) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// bailout: negative number
|
||||
check("void foo(int x) {\n"
|
||||
" int y = -2 | x;\n" // negative number => no error
|
||||
" if (y==1) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// bailout: pass variable to function
|
||||
check("void foo(int x) {\n"
|
||||
" int y = 2 | x;\n"
|
||||
" bar(&y);\n" // pass variable to function => no error
|
||||
" if (y==1) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// no crash on unary operator& (#5643)
|
||||
check("SdrObject* ApplyGraphicToObject() {\n"
|
||||
" if (&rHitObject) {}\n"
|
||||
" else if (rHitObject.IsClosedObj() && !&rHitObject) { }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #5695: increment
|
||||
check("void f(int a0, int n) {\n"
|
||||
" int c = a0 & 3;\n"
|
||||
" for (int a = 0; a < n; a++) {\n"
|
||||
" c++;\n"
|
||||
" if (c == 4)\n"
|
||||
" c = 0;\n"
|
||||
" }\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void mismatchingBitAnd() {
|
||||
check("void f(int a) {\n"
|
||||
" int b = a & 0xf0;\n"
|
||||
" b &= 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching bitmasks. Result is always 0 (X = Y & 0xf0; Z = X & 0x1; => Z=0).\n", errout.str());
|
||||
|
||||
check("void f(int a) {\n"
|
||||
" int b = a & 0xf0;\n"
|
||||
" int c = b & 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Mismatching bitmasks. Result is always 0 (X = Y & 0xf0; Z = X & 0x1; => Z=0).\n", errout.str());
|
||||
|
||||
check("void f(int a) {\n"
|
||||
" int b = a;"
|
||||
" switch (x) {\n"
|
||||
" case 1: b &= 1; break;\n"
|
||||
" case 2: b &= 2; break;\n"
|
||||
" };\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void compare() {
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" if ((x & 4) == 3);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X & 0x4) == 0x3' is always false.\n", errout.str());
|
||||
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" if ((x | 4) == 3);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X | 0x4) == 0x3' is always false.\n", errout.str());
|
||||
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" if ((x | 4) != 3);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X | 0x4) != 0x3' is always true.\n", errout.str());
|
||||
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" if ((x & y & 4 & z ) == 3);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (style) Expression '(X & 0x4) == 0x3' is always false.\n", errout.str());
|
||||
}
|
||||
|
||||
void multicompare() {
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" if (x & 7);\n"
|
||||
" else { if (x == 1); }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str());
|
||||
|
||||
check("void foo(int x)\n"
|
||||
"{\n"
|
||||
" if (x & 7);\n"
|
||||
" else { if (x & 1); }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str());
|
||||
}
|
||||
|
||||
void duplicateIf() {
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (a) { b = 1; }\n"
|
||||
" else { if (a) { b = 2; } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
|
||||
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (a) { b = 1; }\n"
|
||||
" else { if (a) { b = 2; } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
|
||||
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (a == 1) { b = 1; }\n"
|
||||
" else { if (a == 2) { b = 2; }\n"
|
||||
" else { if (a == 1) { b = 3; } } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 2.\n", errout.str());
|
||||
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (a == 1) { b = 1; }\n"
|
||||
" else { if (a == 2) { b = 2; }\n"
|
||||
" else { if (a == 2) { b = 3; } } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4]: (style) Expression is always false because 'else if' condition matches previous condition at line 3.\n", errout.str());
|
||||
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (a++) { b = 1; }\n"
|
||||
" else { if (a++) { b = 2; }\n"
|
||||
" else { if (a++) { b = 3; } } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (!strtok(NULL, \" \")) { b = 1; }\n"
|
||||
" else { if (!strtok(NULL, \" \")) { b = 2; } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int a, int &b) {\n"
|
||||
" x = x / 2;\n"
|
||||
" if (x < 100) { b = 1; }\n"
|
||||
" else { x = x / 2; if (x < 100) { b = 2; } }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int i) {\n"
|
||||
" if(i == 0x02e2000000 || i == 0xa0c6000000)\n"
|
||||
" foo(i);\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// ticket 3689 ( avoid false positive )
|
||||
check("int fitInt(long long int nValue){\n"
|
||||
" if( nValue < 0x7fffffffLL )\n"
|
||||
" {\n"
|
||||
" return 32;\n"
|
||||
" }\n"
|
||||
" if( nValue < 0x7fffffffffffLL )\n"
|
||||
" {\n"
|
||||
" return 48;\n"
|
||||
" }\n"
|
||||
" else {\n"
|
||||
" if( nValue < 0x7fffffffffffffffLL )\n"
|
||||
" {\n"
|
||||
" return 64;\n"
|
||||
" } else\n"
|
||||
" {\n"
|
||||
" return -1;\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(WIDGET *widget) {\n"
|
||||
" if (dynamic_cast<BUTTON*>(widget)){}\n"
|
||||
" else if (dynamic_cast<LABEL*>(widget)){}\n"
|
||||
"}",false);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void invalidMissingSemicolon() {
|
||||
// simply survive - a syntax error would be even better
|
||||
check("void f(int x) {\n"
|
||||
" x = 42\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
};
|
||||
|
||||
REGISTER_TEST(TestAssignIf)
|
File diff suppressed because it is too large
Load Diff
|
@ -36,7 +36,6 @@ private:
|
|||
|
||||
|
||||
void run() {
|
||||
TEST_CASE(oppositeInnerCondition);
|
||||
TEST_CASE(emptyBrackets);
|
||||
|
||||
TEST_CASE(zeroDiv1);
|
||||
|
@ -117,17 +116,6 @@ private:
|
|||
TEST_CASE(trac2071);
|
||||
TEST_CASE(trac2084);
|
||||
TEST_CASE(trac3693);
|
||||
TEST_CASE(modulo);
|
||||
|
||||
TEST_CASE(incorrectLogicOperator1);
|
||||
TEST_CASE(incorrectLogicOperator2);
|
||||
TEST_CASE(incorrectLogicOperator3);
|
||||
TEST_CASE(incorrectLogicOperator4);
|
||||
TEST_CASE(incorrectLogicOperator5); // complex expressions
|
||||
TEST_CASE(incorrectLogicOperator6); // char literals
|
||||
TEST_CASE(incorrectLogicOperator7); // opposite expressions: (expr || !expr)
|
||||
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
|
||||
TEST_CASE(incorrectLogicOp_condSwapping);
|
||||
|
||||
TEST_CASE(memsetZeroBytes);
|
||||
TEST_CASE(memsetInvalid2ndParam);
|
||||
|
@ -137,13 +125,6 @@ private:
|
|||
TEST_CASE(clarifyCalculation);
|
||||
TEST_CASE(clarifyStatement);
|
||||
|
||||
TEST_CASE(clarifyCondition1); // if (a = b() < 0)
|
||||
TEST_CASE(clarifyCondition2); // if (a & b == c)
|
||||
TEST_CASE(clarifyCondition3); // if (! a & b)
|
||||
TEST_CASE(clarifyCondition4); // ticket #3110
|
||||
TEST_CASE(clarifyCondition5); // #3609 CWinTraits<WS_CHILD|WS_VISIBLE>..
|
||||
TEST_CASE(clarifyCondition6); // #3818
|
||||
|
||||
TEST_CASE(duplicateBranch);
|
||||
TEST_CASE(duplicateBranch1); // tests extracted by http://www.viva64.com/en/b/0149/ ( Comparison between PVS-Studio and cppcheck ): Errors detected in Quake 3: Arena by PVS-Studio: Fragement 2
|
||||
TEST_CASE(duplicateBranch2); // empty macro
|
||||
|
@ -267,171 +248,6 @@ private:
|
|||
}
|
||||
|
||||
|
||||
void oppositeInnerCondition() {
|
||||
check("void foo(int a, int b) {\n"
|
||||
" if(a==b)\n"
|
||||
" if(a!=b)\n"
|
||||
" cout << a;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite conditions in nested 'if' blocks lead to a dead code block.\n", errout.str());
|
||||
|
||||
check("void foo(int a, int b) {\n"
|
||||
" if(a==b)\n"
|
||||
" if(b!=a)\n"
|
||||
" cout << a;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite conditions in nested 'if' blocks lead to a dead code block.\n", errout.str());
|
||||
|
||||
check("void foo(int a) {\n"
|
||||
" if(a >= 50) {\n"
|
||||
" if(a < 50)\n"
|
||||
" cout << a;\n"
|
||||
" else\n"
|
||||
" cout << 100;\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Opposite conditions in nested 'if' blocks lead to a dead code block.\n", errout.str());
|
||||
|
||||
// #4186
|
||||
check("void foo(int a) {\n"
|
||||
" if(a >= 50) {\n"
|
||||
" if(a > 50)\n"
|
||||
" cout << a;\n"
|
||||
" else\n"
|
||||
" cout << 100;\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// 4170
|
||||
check("class foo {\n"
|
||||
" void bar() {\n"
|
||||
" if (tok == '(') {\n"
|
||||
" next();\n"
|
||||
" if (tok == ',') {\n"
|
||||
" next();\n"
|
||||
" if (tok != ',') {\n"
|
||||
" op->reg2 = asm_parse_reg();\n"
|
||||
" }\n"
|
||||
" skip(',');\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
" void next();\n"
|
||||
" const char *tok;\n"
|
||||
"};");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void foo(int i)\n"
|
||||
"{\n"
|
||||
" if(i > 5) {\n"
|
||||
" i = bar();\n"
|
||||
" if(i < 5) {\n"
|
||||
" cout << a;\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
|
||||
check("void foo(int& i) {\n"
|
||||
" i=6;\n"
|
||||
"}\n"
|
||||
"void bar(int i) {\n"
|
||||
" if(i>5) {\n"
|
||||
" foo(i);\n"
|
||||
" if(i<5) {\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void foo(int& i);\n"
|
||||
"void bar() {\n"
|
||||
" int i; i = func();\n"
|
||||
" if(i>5) {\n"
|
||||
" foo(i);\n"
|
||||
" if(i<5) {\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void foo(int i);\n"
|
||||
"void bar(int i) {\n"
|
||||
" if(i>5) {\n"
|
||||
" foo(i);\n"
|
||||
" if(i<5) {\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:5]: (warning) Opposite conditions in nested 'if' blocks lead to a dead code block.\n", errout.str());
|
||||
|
||||
check("void foo(int i);\n"
|
||||
"void bar() {\n"
|
||||
" int i; i = func();\n"
|
||||
" if(i>5) {\n"
|
||||
" foo(i);\n"
|
||||
" if(i<5) {\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6]: (warning) Opposite conditions in nested 'if' blocks lead to a dead code block.\n", errout.str());
|
||||
|
||||
// see linux revision 1f80c0cc
|
||||
check("int generic_write_sync(int,int,int);\n"
|
||||
"\n"
|
||||
"void cifs_writev(int i) {\n"
|
||||
" int rc = __generic_file_aio_write();\n"
|
||||
" if (rc > 0){\n"
|
||||
" err = generic_write_sync(file, iocb->ki_pos - rc, rc);\n"
|
||||
" if(rc < 0) {\n" // <- condition is always false
|
||||
" err = rc;\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:7]: (warning) Opposite conditions in nested 'if' blocks lead to a dead code block.\n", errout.str());
|
||||
|
||||
check("void f(struct ABC *abc) {\n"
|
||||
" struct AB *ab = abc->ab;\n"
|
||||
" if (ab->a == 123){\n"
|
||||
" do_something(abc);\n" // might change ab->a
|
||||
" if (ab->a != 123) {\n"
|
||||
" err = rc;\n"
|
||||
" }\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #5731 - fp when undeclared variable is used
|
||||
check("void f() {\n"
|
||||
" if (x == -1){\n"
|
||||
" x = do_something();\n"
|
||||
" if (x != -1) {}\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #5750 - another fp when undeclared variable is used
|
||||
check("void f() {\n"
|
||||
" if (r < w){\n"
|
||||
" r += 3;\n"
|
||||
" if (r > w) {}\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #5874 - array
|
||||
check("void testOppositeConditions2() {\n"
|
||||
" int array[2] = { 0, 0 };\n"
|
||||
" if (array[0] < 2) {\n"
|
||||
" array[0] += 5;\n"
|
||||
" if (array[0] > 2) {}\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void emptyBrackets() {
|
||||
check("{\n"
|
||||
"}");
|
||||
|
@ -3640,547 +3456,6 @@ private:
|
|||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void modulo() {
|
||||
check("bool f(bool& b1, bool& b2, bool& b3) {\n"
|
||||
" b1 = a % 5 == 4;\n"
|
||||
" b2 = a % c == 100000;\n"
|
||||
" b3 = a % 5 == c;\n"
|
||||
" return a % 5 == 5-p;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("bool f(bool& b1, bool& b2, bool& b3, bool& b4, bool& b5) {\n"
|
||||
" b1 = a % 5 < 5;\n"
|
||||
" b2 = a % 5 <= 5;\n"
|
||||
" b3 = a % 5 == 5;\n"
|
||||
" b4 = a % 5 != 5;\n"
|
||||
" b5 = a % 5 >= 5;\n"
|
||||
" return 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"
|
||||
"[test.cpp:5]: (warning) Comparison of modulo result is predetermined, because it is always less than 5.\n"
|
||||
"[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(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());
|
||||
|
||||
check("void f() {\n"
|
||||
" if (a % 2 + b % 2 == 2)\n"
|
||||
" foo();\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator1() {
|
||||
check("void f(int x) {\n"
|
||||
" if ((x != 1) || (x != 3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (1 != x || 3 != x)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 1 || x != 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n" // ast..
|
||||
" if (y == 1 && x == 1 && x == 7) { }\n"
|
||||
"}\n");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 7.\n", errout.str());
|
||||
|
||||
check("void f(int x, int y) {\n"
|
||||
" if (x != 1 || y != 1)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x, int y) {\n"
|
||||
" if ((y == 1) && (x != 1) || (x != 3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x, int y) {\n"
|
||||
" if ((x != 1) || (x != 3) && (y == 1))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x != 1) && (x != 3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x == 1) || (x == 3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x, int y) {\n"
|
||||
" if ((x != 1) || (y != 3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x, int y) {\n"
|
||||
" if ((x != hotdog) || (y != hotdog))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x, int y) {\n"
|
||||
" if ((x != 5) || (y != 5))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x != 5) || (x != 6))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x != 5 || x != 6.\n", errout.str());
|
||||
|
||||
check("void f(unsigned int a, unsigned int b, unsigned int c) {\n"
|
||||
" if((a != b) || (c != b) || (c != a))\n"
|
||||
" {\n"
|
||||
" return true;\n"
|
||||
" }\n"
|
||||
" return false;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator2() {
|
||||
check("void f(float x) {\n"
|
||||
" if ((x == 1) && (x == 1.0))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x == 1) && (x == 0x00000001))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x == 1 && x == 3)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1 && x == 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x == 1.0 && x == 3.0)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
//ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 1.0 && x == 3.0.\n", errout.str());
|
||||
ASSERT_EQUALS("", errout.str()); // float comparisons with == and != are not checked right now - such comparison is a bad idea
|
||||
|
||||
check("void f(float x) {\n"
|
||||
" if (x == 1 && x == 1.0)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void bar(float f) {\n" // #5246
|
||||
" if ((f > 0) && (f < 1)) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 1 && x > 1)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 1.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 1.0 && x > 1.0)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1.0 && x > 1.0.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 1 && x > 1.0)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 1.0.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x >= 1.0 && x <= 1.001)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 1 && x > 3)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
|
||||
|
||||
check("void f(float x) {\n"
|
||||
" if (x < 1.0 && x > 3.0)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1.0 && x > 3.0.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (1 > x && 3 < x)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 3 && x > 1)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 3 || x < 10)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x < 10.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x >= 3 || x <= 10)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x <= 10.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x >= 3 || x < 10)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x < 10.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 3 || x <= 10)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x <= 10.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 3 || x < 3)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x >= 3 || x <= 3)\n"
|
||||
" a++;\n"
|
||||
"}"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x <= 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x >= 3 || x < 3)\n"
|
||||
" a++;\n"
|
||||
"}"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x >= 3 || x < 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 3 || x <= 3)\n"
|
||||
" a++;\n"
|
||||
"}"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x > 3 || x <= 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if((x==3) && (x!=4))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 3, the comparison x != 4 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x!=4) && (x==3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 3, the comparison x != 4 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x==3) || (x!=4))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 3, the comparison x != 4 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x!=4) || (x==3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 3, the comparison x != 4 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x==3) && (x!=3))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x == 3 && x != 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x==6) || (x!=6))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: x == 6 || x != 6.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 10 || x < 3)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 5 && x == 1)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 5 && x == 1.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 5 && x == 6)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 6, the comparison x > 5 is always true.\n", errout.str());
|
||||
|
||||
// #3419
|
||||
check("void f() {\n"
|
||||
" if ( &q != &a && &q != &b ) { }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
// #3676
|
||||
check("void f(int m_x2, int w, int x) {\n"
|
||||
" if (x + w - 1 > m_x2 || m_x2 < 0 )\n"
|
||||
" m_x2 = x + w - 1;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(float x) {\n" // x+1 => x
|
||||
" if (x <= 1.0e20 && x >= -1.0e20) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(float x) {\n" // x+1 => x
|
||||
" if (x >= 1.0e20 && x <= 1.0e21) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(float x) {\n" // x+1 => x
|
||||
" if (x <= -1.0e20 && x >= -1.0e21) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator3() {
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 5 && x == 1;\n"
|
||||
" c = x < 1 && x == 3;\n"
|
||||
" d = x >= 5 && x == 1;\n"
|
||||
" e = x <= 1 && x == 3;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 5 && x == 1.\n"
|
||||
"[test.cpp:3]: (warning) Logical conjunction always evaluates to false: x < 1 && x == 3.\n"
|
||||
"[test.cpp:4]: (warning) Logical conjunction always evaluates to false: x >= 5 && x == 1.\n"
|
||||
"[test.cpp:5]: (warning) Logical conjunction always evaluates to false: x <= 1 && x == 3.\n", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator4() {
|
||||
check("void f(int x) {\n"
|
||||
" if (x && x != $0) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator5() { // complex expressions
|
||||
check("void f(int x) {\n"
|
||||
" if (x+3 > 2 || x+3 < 10) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: EXPR > 2 || EXPR < 10.\n", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator6() { // char literals
|
||||
check("void f(char x) {\n"
|
||||
" if (x == '1' || x == '2') {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(char x) {\n"
|
||||
" if (x == '1' && x == '2') {}\n"
|
||||
"}");
|
||||
TODO_ASSERT_EQUALS("error", "", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOperator7() { // opposite expressions
|
||||
check("void f(int i) {\n"
|
||||
" if (i || !i) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical disjunction always evaluates to true: i||!i.\n", errout.str());
|
||||
}
|
||||
|
||||
void secondAlwaysTrueFalseWhenFirstTrueError() {
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 5 && x != 1)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 5 && x != 6)\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x > 5) && (x != 1))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x > 5) && (x != 6))\n"
|
||||
" a++;\n"
|
||||
"}\n"
|
||||
);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 3 || x == 4;\n"
|
||||
" c = x < 5 || x == 4;\n"
|
||||
" d = x >= 3 || x == 4;\n"
|
||||
" e = x <= 5 || x == 4;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x == 4, the comparison x > 3 is always true.\n"
|
||||
"[test.cpp:3]: (style) Redundant condition: If x == 4, the comparison x < 5 is always true.\n"
|
||||
"[test.cpp:4]: (style) Redundant condition: If x == 4, the comparison x >= 3 is always true.\n"
|
||||
"[test.cpp:5]: (style) Redundant condition: If x == 4, the comparison x <= 5 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 5 || x != 1;\n"
|
||||
" c = x < 1 || x != 3;\n"
|
||||
" d = x >= 5 || x != 1;\n"
|
||||
" e = x <= 1 || x != 3;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 5, the comparison x != 1 is always true.\n"
|
||||
"[test.cpp:3]: (style) Redundant condition: If x < 1, the comparison x != 3 is always true.\n"
|
||||
"[test.cpp:4]: (style) Redundant condition: If x >= 5, the comparison x != 1 is always true.\n"
|
||||
"[test.cpp:5]: (style) Redundant condition: If x <= 1, the comparison x != 3 is always true.\n", errout.str());
|
||||
|
||||
check("void f(int x, bool& b) {\n"
|
||||
" b = x > 6 && x > 5;\n"
|
||||
" c = x > 5 || x > 6;\n"
|
||||
" d = x < 6 && x < 5;\n"
|
||||
" e = x < 5 || x < 6;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Redundant condition: If x > 6, the comparison x > 5 is always true.\n"
|
||||
"[test.cpp:3]: (style) Redundant condition: If x > 6, the comparison x > 5 is always true.\n"
|
||||
"[test.cpp:4]: (style) Redundant condition: If x < 5, the comparison x < 6 is always true.\n"
|
||||
"[test.cpp:5]: (style) Redundant condition: If x < 5, the comparison x < 6 is always true.\n", errout.str());
|
||||
}
|
||||
|
||||
void incorrectLogicOp_condSwapping() {
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 1 && x > 3)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (1 > x && x > 3)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x < 1 && 3 < x)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (1 > x && 3 < x)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x < 1 && x > 3.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 3 && x < 1)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 3 && x < 1.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (3 < x && x < 1)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 3 && x < 1.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (x > 3 && 1 > x)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 3 && x < 1.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if (3 < x && 1 > x)\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (warning) Logical conjunction always evaluates to false: x > 3 && x < 1.\n", errout.str());
|
||||
}
|
||||
|
||||
void memsetZeroBytes() {
|
||||
check("void f() {\n"
|
||||
" memset(p, 10, 0x0);\n"
|
||||
|
@ -4395,97 +3670,6 @@ private:
|
|||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
// clarify conditions with = and comparison
|
||||
void clarifyCondition1() {
|
||||
check("void f() {\n"
|
||||
" if (x = b() < 0) {}\n" // don't simplify and verify this code
|
||||
"}", 0, false, false, false, false);
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (assignment + comparison); Clarify expression with parentheses.\n", errout.str());
|
||||
|
||||
check("void f(int i) {\n"
|
||||
" for (i = 0; i < 10; i++) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" x = a<int>(); if (x) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
}
|
||||
|
||||
// clarify conditions with bitwise operator and comparison
|
||||
void clarifyCondition2() {
|
||||
check("void f() {\n"
|
||||
" if (x & 3 == 2) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" if (a & fred1.x == fred2.y) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Suspicious condition (bitwise operator + comparison); Clarify expression with parentheses.\n", errout.str());
|
||||
}
|
||||
|
||||
// clarify condition that uses ! operator and then bitwise operator
|
||||
void clarifyCondition3() {
|
||||
check("void f(int w) {\n"
|
||||
" if(!w & 0x8000) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Boolean result is used in bitwise operation. Clarify expression with parentheses.\n", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" if (x == foo() & 2) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2]: (style) Boolean result is used in bitwise operation. Clarify expression with parentheses.\n", errout.str());
|
||||
|
||||
check("void f(std::list<int> &ints) { }");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f() { A<x &> a; }");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f() { a(x<y|z,0); }", "test.c"); // filename is c => there are never templates
|
||||
ASSERT_EQUALS("[test.c:1]: (style) Boolean result is used in bitwise operation. Clarify expression with parentheses.\n", errout.str());
|
||||
|
||||
check("class A<B&,C>;", "test.C");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
|
||||
check("void f() {\n"
|
||||
" if (result != (char *)&inline_result) { }\n" // don't simplify and verify cast
|
||||
"}", 0, false, false, false, false);
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void clarifyCondition4() { // ticket #3110
|
||||
check("typedef double SomeType;\n"
|
||||
"typedef std::pair<std::string,SomeType> PairType;\n"
|
||||
"struct S\n"
|
||||
"{\n"
|
||||
" bool operator()\n"
|
||||
" ( PairType const & left\n"
|
||||
" , PairType const & right) const\n"
|
||||
" {\n"
|
||||
" return left.first < right.first;\n"
|
||||
" }\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void clarifyCondition5() { // ticket #3609 (using | in template instantiation)
|
||||
check("CWinTraits<WS_CHILD|WS_VISIBLE>::GetWndStyle(0);");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void clarifyCondition6() {
|
||||
check("template<class Y>\n"
|
||||
"SharedPtr& operator=( SharedPtr<Y> const & r ) {\n"
|
||||
" px = r.px;\n"
|
||||
" return *this;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("", errout.str());
|
||||
}
|
||||
|
||||
void duplicateBranch() {
|
||||
check("void f(int a, int &b) {\n"
|
||||
" if (a)\n"
|
||||
|
@ -4754,6 +3938,12 @@ private:
|
|||
" if ((b + a) > (a + b)) {}\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '>'.\n", errout.str());
|
||||
|
||||
check("void f(int x) {\n"
|
||||
" if ((x == 1) && (x == 0x00000001))\n"
|
||||
" a++;\n"
|
||||
"}");
|
||||
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Same expression on both sides of '&&'.\n", errout.str());
|
||||
}
|
||||
|
||||
void duplicateExpression2() { // check if float is NaN or Inf
|
||||
|
|
|
@ -32,7 +32,6 @@
|
|||
<ClCompile Include="options.cpp" />
|
||||
<ClCompile Include="test64bit.cpp" />
|
||||
<ClCompile Include="testassert.cpp" />
|
||||
<ClCompile Include="testassignif.cpp" />
|
||||
<ClCompile Include="testautovariables.cpp" />
|
||||
<ClCompile Include="testbool.cpp" />
|
||||
<ClCompile Include="testboost.cpp" />
|
||||
|
@ -40,6 +39,7 @@
|
|||
<ClCompile Include="testcharvar.cpp" />
|
||||
<ClCompile Include="testclass.cpp" />
|
||||
<ClCompile Include="testcmdlineparser.cpp" />
|
||||
<ClCompile Include="testcondition.cpp" />
|
||||
<ClCompile Include="testconstructors.cpp" />
|
||||
<ClCompile Include="testcppcheck.cpp" />
|
||||
<ClCompile Include="testdivision.cpp" />
|
||||
|
|
|
@ -25,9 +25,6 @@
|
|||
<ClCompile Include="test64bit.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="testassignif.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="testautovariables.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
|
@ -190,6 +187,9 @@
|
|||
<ClCompile Include="teststring.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
<ClCompile Include="testcondition.cpp">
|
||||
<Filter>Source Files</Filter>
|
||||
</ClCompile>
|
||||
</ItemGroup>
|
||||
<ItemGroup>
|
||||
<ClInclude Include="options.h">
|
||||
|
|
Loading…
Reference in New Issue