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

Refactoring to address some issues from #5895 (handling of unsigned numbers). Also adding a TODO testcase since the real issue (FP) is still not fixed

This commit is contained in:
Alexander Mai 2015-06-13 18:08:13 +02:00
parent 40d7baa6bb
commit a7b82b5c28
5 changed files with 42 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Makefile
View File

@ -214,6 +214,7 @@ TESTOBJ = test/options.o \
test/testtimer.o \
test/testtoken.o \
test/testtokenize.o \
test/testtokenlist.o \
test/testtype.o \
test/testuninitvar.o \
test/testunusedfunctions.o \
@ -573,6 +574,9 @@ test/testtoken.o: test/testtoken.cpp lib/cxx11emu.h test/testsuite.h lib/errorlo
test/testtokenize.o: test/testtokenize.cpp lib/cxx11emu.h test/testsuite.h lib/errorlogger.h lib/config.h lib/suppressions.h test/redirect.h lib/library.h lib/mathlib.h lib/token.h lib/valueflow.h lib/tokenize.h lib/tokenlist.h lib/settings.h lib/standards.h lib/timer.h lib/path.h lib/preprocessor.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testtokenize.o test/testtokenize.cpp
test/testtokenlist.o: test/testtokenlist.cpp lib/cxx11emu.h test/testsuite.h lib/errorlogger.h lib/config.h lib/suppressions.h test/redirect.h lib/library.h lib/mathlib.h lib/token.h lib/valueflow.h lib/settings.h lib/standards.h lib/timer.h lib/tokenlist.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testtokenlist.o test/testtokenlist.cpp
test/testtype.o: test/testtype.cpp lib/cxx11emu.h lib/preprocessor.h lib/config.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/symboldatabase.h lib/token.h lib/valueflow.h lib/mathlib.h lib/checktype.h lib/check.h lib/settings.h lib/library.h lib/standards.h lib/timer.h test/testsuite.h test/redirect.h test/testutils.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CFG) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -std=c++0x -c -o test/testtype.o test/testtype.cpp

36
lib/checkcondition.cpp
View File

@ -525,7 +525,8 @@ static std::string invertOperatorForOperandSwap(std::string s)
return s;
}
static bool checkIntRelation(const std::string &op, const MathLib::bigint value1, const MathLib::bigint value2)
template <typename T>
static bool checkIntRelation(const std::string &op, const T value1, const T value2)
{
return (op == "==" && value1 == value2) ||
(op == "!=" && value1 != value2) ||
@ -543,7 +544,9 @@ static bool checkFloatRelation(const std::string &op, const double value1, const
(op == "<=" && value1 <= value2);
}
template<class T> static T getvalue(const int test, const T value1, const T value2)
template<class T>
static inline T getvalue(const int test, const T value1, const T value2)
{
// test:
// 1 => return value that is less than both value1 and value2
@ -553,7 +556,7 @@ template<class T> static T getvalue(const int test, const T value1, const T valu
// 5 => return value that is larger than both value1 and value2
switch (test) {
case 1: {
T ret = std::min(value1, value2);
const T ret = std::min(value1, value2);
if ((ret - (T)1) < ret)
return ret - (T)1;
else if ((ret / (T)2) < ret)
@ -565,11 +568,19 @@ template<class T> static T getvalue(const int test, const T value1, const T valu
case 2:
return value1;
case 3:
return (value1 + value2) / (T)2;
if (std::numeric_limits<T>::is_integer) {
const T min = std::min(value1, value2);
if (min== std::numeric_limits<T>::max())
return min;
else
return min+1; // see #5895
} else {
return (value1 + value2) / (T)2;
}
case 4:
return value2;
case 5: {
T ret = std::max(value1, value2);
const T ret = std::max(value1, value2);
if ((ret + (T)1) > ret)
return ret + (T)1;
else if ((ret / (T)2) > ret)
@ -672,6 +683,13 @@ void CheckCondition::checkIncorrectLogicOperator()
if (isfloat && (op1 == "==" || op1 == "!=" || op2 == "==" || op2 == "!="))
continue;
const double d1 = (isfloat) ? MathLib::toDoubleNumber(value1) : 0;
const double d2 = (isfloat) ? MathLib::toDoubleNumber(value2) : 0;
const MathLib::bigint i1 = (isfloat) ? 0 : MathLib::toLongNumber(value1);
const MathLib::bigint i2 = (isfloat) ? 0 : MathLib::toLongNumber(value2);
const bool useUnsignedInt = (std::numeric_limits<MathLib::bigint>::max()==i1)||(std::numeric_limits<MathLib::bigint>::max()==i2);
const MathLib::biguint u1 = (useUnsignedInt) ? MathLib::toLongNumber(value1) : 0;
const MathLib::biguint u2 = (useUnsignedInt) ? MathLib::toLongNumber(value2) : 0;
// evaluate if expression is always true/false
bool alwaysTrue = true, alwaysFalse = true;
bool firstTrue = true, secondTrue = true;
@ -684,14 +702,14 @@ void CheckCondition::checkIncorrectLogicOperator()
// 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 if (useUnsignedInt) {
const MathLib::biguint testvalue = getvalue<MathLib::biguint>(test, u1, u2);
result1 = checkIntRelation(op1, testvalue, u1);
result2 = checkIntRelation(op2, testvalue, u2);
} 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);

2
lib/tokenlist.cpp
View File

@ -132,7 +132,7 @@ void TokenList::addtoken(const std::string & str, const unsigned int lineno, con
std::string str2;
if (MathLib::isHex(str) || MathLib::isOct(str) || MathLib::isBin(str)) {
std::ostringstream str2stream;
str2stream << MathLib::toLongNumber(str);
str2stream << MathLib::MathLib::toULongNumber(str);
str2 = str2stream.str();
} else if (str.compare(0, 5, "_Bool") == 0) {
str2 = "bool";

10
test/testcondition.cpp
View File

@ -48,6 +48,7 @@ private:
TEST_CASE(incorrectLogicOperator7); // opposite expressions: (expr || !expr)
TEST_CASE(secondAlwaysTrueFalseWhenFirstTrueError);
TEST_CASE(incorrectLogicOp_condSwapping);
TEST_CASE(testBug5895);
TEST_CASE(modulo);
@ -656,7 +657,6 @@ private:
" if (x == 1.0 && x == 3.0)\n"
" a++;\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"
@ -1408,6 +1408,14 @@ private:
"}");
ASSERT_EQUALS("", errout.str());
}
void testBug5895() {
check("void png_parse(uint64_t init, int buf_size) {\n"
" if (init == 0x89504e470d0a1a0a || init == 0x8a4d4e470d0a1a0a)\n"
" ;\n"
"}");
TODO_ASSERT_EQUALS("", "[test.cpp:2]: (style) Redundant condition: If init == 9894494448401390090, the comparison init == 9965707617509186058 is always true.\n", errout.str());
}
};
REGISTER_TEST(TestCondition)

1
test/testfiles.pri
View File

@ -50,6 +50,7 @@ SOURCES += $${BASEPATH}/test64bit.cpp \
$${BASEPATH}/testtimer.cpp \
$${BASEPATH}/testtoken.cpp \
$${BASEPATH}/testtokenize.cpp \
$${BASEPATH}/testtokenlist.cpp \
$${BASEPATH}/testtype.cpp \
$${BASEPATH}/testuninitvar.cpp \
$${BASEPATH}/testunusedfunctions.cpp \