Restored (intentional) content of screwed up commits 1bcdf4ce3d
and 674f7980d519712ff16d8f874dfe55a84deb4b5b:
- New check (Inconclusive): Array filled incompletely with memset/memcpy/memmove -- This check only warns if the number of elements is given as size in bytes to memset, memcpy or memmove and if the size of an element is larger than 1 Byte. It does not warn for random numbers - New check: Detect ineffective statements like '*foo++;' (Should be: '(*foo)++;') Sorry for the inconveniences.
This commit is contained in:
parent
0f1accc2da
commit
76fbcce13f
|
@ -229,6 +229,41 @@ void CheckOther::clarifyConditionError(const Token *tok, bool assign, bool boolo
|
||||||
errmsg);
|
errmsg);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
//---------------------------------------------------------------------------
|
||||||
|
// Clarify (meaningless) statements like *foo++; with parantheses.
|
||||||
|
//---------------------------------------------------------------------------
|
||||||
|
void CheckOther::clarifyStatement()
|
||||||
|
{
|
||||||
|
if (!_settings->isEnabled("style"))
|
||||||
|
return;
|
||||||
|
|
||||||
|
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
|
||||||
|
if (Token::Match(tok, "[{};] * %var%")) {
|
||||||
|
tok = tok->tokAt(3);
|
||||||
|
while (tok) {
|
||||||
|
if (tok->str() == "[")
|
||||||
|
tok = tok->link()->next();
|
||||||
|
if (Token::Match(tok, ".|:: %var%")) {
|
||||||
|
if (tok->strAt(2) == "(")
|
||||||
|
tok = tok->linkAt(2)->next();
|
||||||
|
else
|
||||||
|
tok = tok->tokAt(2);
|
||||||
|
} else
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if (Token::Match(tok, "++|-- [;,]"))
|
||||||
|
clarifyStatementError(tok);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckOther::clarifyStatementError(const Token *tok)
|
||||||
|
{
|
||||||
|
reportError(tok, Severity::warning, "clarifyStatement", "Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n"
|
||||||
|
"A statement like '*A++;' might not do what you intended. 'operator*' is executed before postfix 'operator++'. "
|
||||||
|
"Thus, the dereference is meaningless. Did you intend to write '(*A)++;'?");
|
||||||
|
}
|
||||||
|
|
||||||
//---------------------------------------------------------------------------
|
//---------------------------------------------------------------------------
|
||||||
// if (bool & bool) -> if (bool && bool)
|
// if (bool & bool) -> if (bool && bool)
|
||||||
// if (bool | bool) -> if (bool || bool)
|
// if (bool | bool) -> if (bool || bool)
|
||||||
|
@ -3116,3 +3151,43 @@ void CheckOther::negativeBitwiseShiftError(const Token *tok)
|
||||||
{
|
{
|
||||||
reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value.");
|
reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value.");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
//---------------------------------------------------------------------------
|
||||||
|
// Check for incompletely filled buffers.
|
||||||
|
//---------------------------------------------------------------------------
|
||||||
|
void CheckOther::checkIncompleteArrayFill()
|
||||||
|
{
|
||||||
|
if (!_settings->inconclusive || !_settings->isEnabled("style"))
|
||||||
|
return;
|
||||||
|
|
||||||
|
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
|
||||||
|
|
||||||
|
for (const Token* tok = _tokenizer->list.front(); tok; tok = tok->next()) {
|
||||||
|
if (Token::Match(tok, "memset|memcpy|memmove ( %var% ,") && Token::Match(tok->linkAt(1)->tokAt(-2), ", %num% )")) {
|
||||||
|
const Variable* var = symbolDatabase->getVariableFromVarId(tok->tokAt(2)->varId());
|
||||||
|
if (!var || !var->isArray() || var->dimensions().empty() || !var->dimension(0))
|
||||||
|
continue;
|
||||||
|
|
||||||
|
if (MathLib::toLongNumber(tok->linkAt(1)->strAt(-1)) == var->dimension(0)) {
|
||||||
|
unsigned int size = _tokenizer->sizeOfType(var->typeStartToken());
|
||||||
|
if ((size != 1 && size != 100 && size != 0) || Token::Match(var->typeEndToken(), "*"))
|
||||||
|
incompleteArrayFillError(tok, var->name(), tok->str(), false);
|
||||||
|
else if (var->typeStartToken()->str() == "bool" && _settings->isEnabled("portability")) // sizeof(bool) is not 1 on all platforms
|
||||||
|
incompleteArrayFillError(tok, var->name(), tok->str(), true);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void CheckOther::incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean)
|
||||||
|
{
|
||||||
|
if (boolean)
|
||||||
|
reportError(tok, Severity::portability, "incompleteArrayFill",
|
||||||
|
"Array '" + buffer + "' might be filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n"
|
||||||
|
"The array '" + buffer + "' is filled incompletely. The function '" + function + "()' needs the size given in bytes, but the type 'bool' is larger than 1 on some platforms. Did you forget to multiply the size with 'sizeof(*" + buffer + ")'?", true);
|
||||||
|
else
|
||||||
|
reportError(tok, Severity::warning, "incompleteArrayFill",
|
||||||
|
"Array '" + buffer + "' is filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n"
|
||||||
|
"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);
|
||||||
|
}
|
||||||
|
|
|
@ -74,6 +74,7 @@ public:
|
||||||
checkOther.clarifyCondition(); // not simplified because ifAssign
|
checkOther.clarifyCondition(); // not simplified because ifAssign
|
||||||
checkOther.checkComparisonOfBoolExpressionWithInt();
|
checkOther.checkComparisonOfBoolExpressionWithInt();
|
||||||
checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574)
|
checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574)
|
||||||
|
checkOther.checkIncompleteArrayFill();
|
||||||
}
|
}
|
||||||
|
|
||||||
/** @brief Run checks against the simplified token list */
|
/** @brief Run checks against the simplified token list */
|
||||||
|
@ -82,6 +83,7 @@ public:
|
||||||
|
|
||||||
// Checks
|
// Checks
|
||||||
checkOther.clarifyCalculation();
|
checkOther.clarifyCalculation();
|
||||||
|
checkOther.clarifyStatement();
|
||||||
checkOther.checkConstantFunctionParameter();
|
checkOther.checkConstantFunctionParameter();
|
||||||
checkOther.checkIncompleteStatement();
|
checkOther.checkIncompleteStatement();
|
||||||
|
|
||||||
|
@ -113,6 +115,9 @@ public:
|
||||||
/** @brief Suspicious condition (assignment+comparison) */
|
/** @brief Suspicious condition (assignment+comparison) */
|
||||||
void clarifyCondition();
|
void clarifyCondition();
|
||||||
|
|
||||||
|
/** @brief Suspicious statement like '*A++;' */
|
||||||
|
void clarifyStatement();
|
||||||
|
|
||||||
/** @brief Are there C-style pointer casts in a c++ file? */
|
/** @brief Are there C-style pointer casts in a c++ file? */
|
||||||
void warningOldStylePointerCast();
|
void warningOldStylePointerCast();
|
||||||
|
|
||||||
|
@ -244,10 +249,14 @@ public:
|
||||||
/** @brief %Check for bitwise operation with negative right operand */
|
/** @brief %Check for bitwise operation with negative right operand */
|
||||||
void checkNegativeBitwiseShift();
|
void checkNegativeBitwiseShift();
|
||||||
|
|
||||||
|
/** @brief %Check for buffers that are filled incompletely with memset and similar functions */
|
||||||
|
void checkIncompleteArrayFill();
|
||||||
|
|
||||||
private:
|
private:
|
||||||
// Error messages..
|
// Error messages..
|
||||||
void clarifyCalculationError(const Token *tok, const std::string &op);
|
void clarifyCalculationError(const Token *tok, const std::string &op);
|
||||||
void clarifyConditionError(const Token *tok, bool assign, bool boolop);
|
void clarifyConditionError(const Token *tok, bool assign, bool boolop);
|
||||||
|
void clarifyStatementError(const Token* tok);
|
||||||
void sizeofsizeofError(const Token *tok);
|
void sizeofsizeofError(const Token *tok);
|
||||||
void sizeofCalculationError(const Token *tok, bool inconclusive);
|
void sizeofCalculationError(const Token *tok, bool inconclusive);
|
||||||
void cstyleCastError(const Token *tok);
|
void cstyleCastError(const Token *tok);
|
||||||
|
@ -302,6 +311,7 @@ private:
|
||||||
void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
|
void moduloAlwaysTrueFalseError(const Token* tok, const std::string& maxVal);
|
||||||
void negativeBitwiseShiftError(const Token *tok);
|
void negativeBitwiseShiftError(const Token *tok);
|
||||||
void redundantCopyError(const Token *tok, const std::string &varname);
|
void redundantCopyError(const Token *tok, const std::string &varname);
|
||||||
|
void incompleteArrayFillError(const Token* tok, const std::string& buffer, const std::string& function, bool boolean);
|
||||||
|
|
||||||
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
|
||||||
CheckOther c(0, settings, errorLogger);
|
CheckOther c(0, settings, errorLogger);
|
||||||
|
@ -344,6 +354,7 @@ private:
|
||||||
c.memsetZeroBytesError(0, "varname");
|
c.memsetZeroBytesError(0, "varname");
|
||||||
c.clarifyCalculationError(0, "+");
|
c.clarifyCalculationError(0, "+");
|
||||||
c.clarifyConditionError(0, true, false);
|
c.clarifyConditionError(0, true, false);
|
||||||
|
c.clarifyStatementError(0);
|
||||||
c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
|
c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12");
|
||||||
c.incorrectStringBooleanError(0, "\"Hello World\"");
|
c.incorrectStringBooleanError(0, "\"Hello World\"");
|
||||||
c.incrementBooleanError(0);
|
c.incrementBooleanError(0);
|
||||||
|
@ -364,6 +375,7 @@ private:
|
||||||
c.SuspiciousSemicolonError(0);
|
c.SuspiciousSemicolonError(0);
|
||||||
c.cctypefunctionCallError(0, "funname", "value");
|
c.cctypefunctionCallError(0, "funname", "value");
|
||||||
c.moduloAlwaysTrueFalseError(0, "1");
|
c.moduloAlwaysTrueFalseError(0, "1");
|
||||||
|
c.incompleteArrayFillError(0, "buffer", "memset", false);
|
||||||
}
|
}
|
||||||
|
|
||||||
static std::string myName() {
|
static std::string myName() {
|
||||||
|
@ -422,7 +434,8 @@ private:
|
||||||
"* using bool in bitwise expression\n"
|
"* using bool in bitwise expression\n"
|
||||||
"* Suspicious use of ; at the end of 'if/for/while' statement.\n"
|
"* Suspicious use of ; at the end of 'if/for/while' statement.\n"
|
||||||
"* incorrect usage of functions from ctype library.\n"
|
"* incorrect usage of functions from ctype library.\n"
|
||||||
"* Comparisons of modulo results that are always true/false.\n";
|
"* Comparisons of modulo results that are always true/false.\n"
|
||||||
|
"* Array filled incompletely using memset/memcpy/memmove.\n";
|
||||||
}
|
}
|
||||||
|
|
||||||
void checkExpressionRange(const std::list<const Function*> &constFunctions,
|
void checkExpressionRange(const std::list<const Function*> &constFunctions,
|
||||||
|
|
|
@ -119,6 +119,7 @@ private:
|
||||||
TEST_CASE(sizeofForNumericParameter);
|
TEST_CASE(sizeofForNumericParameter);
|
||||||
|
|
||||||
TEST_CASE(clarifyCalculation);
|
TEST_CASE(clarifyCalculation);
|
||||||
|
TEST_CASE(clarifyStatement);
|
||||||
|
|
||||||
TEST_CASE(clarifyCondition1); // if (a = b() < 0)
|
TEST_CASE(clarifyCondition1); // if (a = b() < 0)
|
||||||
TEST_CASE(clarifyCondition2); // if (a & b == c)
|
TEST_CASE(clarifyCondition2); // if (a & b == c)
|
||||||
|
@ -164,6 +165,8 @@ private:
|
||||||
TEST_CASE(checkRedundantCopy);
|
TEST_CASE(checkRedundantCopy);
|
||||||
|
|
||||||
TEST_CASE(checkNegativeShift);
|
TEST_CASE(checkNegativeShift);
|
||||||
|
|
||||||
|
TEST_CASE(incompleteArrayFill);
|
||||||
}
|
}
|
||||||
|
|
||||||
void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) {
|
void check(const char code[], const char *filename = NULL, bool experimental = false, bool inconclusive = true) {
|
||||||
|
@ -172,6 +175,7 @@ private:
|
||||||
|
|
||||||
Settings settings;
|
Settings settings;
|
||||||
settings.addEnabled("style");
|
settings.addEnabled("style");
|
||||||
|
settings.addEnabled("portability");
|
||||||
settings.inconclusive = inconclusive;
|
settings.inconclusive = inconclusive;
|
||||||
settings.experimental = experimental;
|
settings.experimental = experimental;
|
||||||
|
|
||||||
|
@ -3942,6 +3946,46 @@ private:
|
||||||
ASSERT_EQUALS("", errout.str());
|
ASSERT_EQUALS("", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void clarifyStatement() {
|
||||||
|
check("char* f(char* c) {\n"
|
||||||
|
" *c++;\n"
|
||||||
|
" return c;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
|
||||||
|
|
||||||
|
check("char* f(char** c) {\n"
|
||||||
|
" *c[5]--;\n"
|
||||||
|
" return *c;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
|
||||||
|
|
||||||
|
check("void f(Foo f) {\n"
|
||||||
|
" *f.a++;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
|
||||||
|
|
||||||
|
check("void f(Foo f) {\n"
|
||||||
|
" *f.a[5].v[3]++;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
|
||||||
|
|
||||||
|
check("void f(Foo f) {\n"
|
||||||
|
" *f.a(1, 5).v[x + y]++;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:2]: (warning) Ineffective statement similar to '*A++;'. Did you intend to write '(*A)++;'?\n", errout.str());
|
||||||
|
|
||||||
|
check("char* f(char* c) {\n"
|
||||||
|
" (*c)++;\n"
|
||||||
|
" return c;\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f(char* c) {\n"
|
||||||
|
" bar(*c++);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
}
|
||||||
|
|
||||||
// clarify conditions with = and comparison
|
// clarify conditions with = and comparison
|
||||||
void clarifyCondition1() {
|
void clarifyCondition1() {
|
||||||
check("void f() {\n"
|
check("void f() {\n"
|
||||||
|
@ -5837,6 +5881,67 @@ private:
|
||||||
"}");
|
"}");
|
||||||
ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str());
|
ASSERT_EQUALS("[test.cpp:4]: (error) Shifting by a negative value.\n", errout.str());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void incompleteArrayFill() {
|
||||||
|
check("void f() {\n"
|
||||||
|
" int a[5];\n"
|
||||||
|
" memset(a, 123, 5);\n"
|
||||||
|
" memcpy(a, b, 5);\n"
|
||||||
|
" memmove(a, b, 5);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n"
|
||||||
|
"[test.cpp:4]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memcpy()' with 'sizeof(*a)'?\n"
|
||||||
|
"[test.cpp:5]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memmove()' with 'sizeof(*a)'?\n", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" Foo* a[5];\n"
|
||||||
|
" memset(a, 'a', 5);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", errout.str());
|
||||||
|
|
||||||
|
check("class Foo {int a; int b;};\n"
|
||||||
|
"void f() {\n"
|
||||||
|
" Foo a[5];\n"
|
||||||
|
" memset(a, 'a', 5);\n"
|
||||||
|
"}");
|
||||||
|
TODO_ASSERT_EQUALS("[test.cpp:3]: (warning, inconclusive) Array 'a' is filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", "", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" Foo a[5];\n" // Size of foo is unknown
|
||||||
|
" memset(a, 'a', 5);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" char a[5];\n"
|
||||||
|
" memset(a, 'a', 5);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" int a[5];\n"
|
||||||
|
" memset(a+15, 'a', 5);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" bool a[5];\n"
|
||||||
|
" memset(a, false, 5*sizeof(bool));\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" bool a[5];\n"
|
||||||
|
" memset(a, false, 5*sizeof(*a));\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("", errout.str());
|
||||||
|
|
||||||
|
check("void f() {\n"
|
||||||
|
" bool a[5];\n"
|
||||||
|
" memset(a, false, 5);\n"
|
||||||
|
"}");
|
||||||
|
ASSERT_EQUALS("[test.cpp:3]: (portability, inconclusive) Array 'a' might be filled incompletely. Did you forget to multiply the size given to 'memset()' with 'sizeof(*a)'?\n", errout.str());
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
REGISTER_TEST(TestOther)
|
REGISTER_TEST(TestOther)
|
||||||
|
|
Loading…
Reference in New Issue