diff --git a/lib/checktype.cpp b/lib/checktype.cpp index 21652f5c3..5d0e17911 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -243,14 +243,17 @@ void CheckType::checkSignConversion() void CheckType::signConversionError(const Token *tok, const bool constvalue) { - const std::string varname(tok ? tok->str() : "var"); + const std::string expr(tok ? tok->expressionString() : "var"); - reportError(tok, - Severity::warning, - "signConversion", - (constvalue) ? - "$symbol:" + varname + "\nSuspicious code: sign conversion of $symbol in calculation because '$symbol' has a negative value" : - "$symbol:" + varname + "\nSuspicious code: sign conversion of $symbol in calculation, even though $symbol can have a negative value", CWE195, false); + std::ostringstream msg; + if (tok && tok->isName()) + msg << "$symbol:" << expr << "\n"; + if (constvalue) + msg << "Suspicious code: sign conversion of '" << expr << "' in calculation because '" << expr << "' has a negative value"; + else + msg << "Suspicious code: sign conversion of '" << expr << "' in calculation, even though '" << expr << "' can have a negative value"; + + reportError(tok, Severity::warning, "signConversion", msg.str(), CWE195, false); } diff --git a/test/testtype.cpp b/test/testtype.cpp index 0695ec433..7c14edd30 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -188,19 +188,25 @@ private: void signConversion() { check("x = -4 * (unsigned)y;"); - ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of -4 in calculation because '-4' has a negative value\n", errout.str()); + ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of '-4' in calculation because '-4' has a negative value\n", errout.str()); + + check("unsigned int dostuff(int x) {\n" // x is signed + " if (x==0) {}\n" + " return (x-1)*sizeof(int);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (warning) Suspicious code: sign conversion of 'x-1' in calculation, even though 'x-1' can have a negative value\n", errout.str()); check("unsigned int f1(signed int x, unsigned int y) {" // x is signed " return x * y;\n" "}\n" "void f2() { f1(-4,4); }"); - TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", "", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of 'x' in calculation, even though x can have a negative value\n", "", errout.str()); check("unsigned int f1(int x) {" // x has no signedness, but it can have the value -1 so assume it's signed " return x * 5U;\n" "}\n" "void f2() { f1(-4); }"); - TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of x in calculation, even though x can have a negative value\n", "", errout.str()); + TODO_ASSERT_EQUALS("[test.cpp:1]: (warning) Suspicious code: sign conversion of 'x' in calculation, even though x can have a negative value\n", "", errout.str()); check("unsigned int f1(int x) {" // #6168: FP for inner calculation " return 5U * (1234 - x);\n" // <- signed subtraction, x is not sign converted @@ -218,7 +224,7 @@ private: check("size_t foo(size_t x) {\n" " return -2 * x;\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious code: sign conversion of -2 in calculation because '-2' has a negative value\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2]: (warning) Suspicious code: sign conversion of '-2' in calculation because '-2' has a negative value\n", errout.str()); } void longCastAssign() {