Fixed #1132 (Detection of misused scope objects)

Emits error in the form:

    [useless_lock.cpp:18]: (error) instance of "Lock" object destroyed immediately

...if an instance of a class or struct is unnamed and therefore destroyed
straight after creation.

Removed "internal error" from token.cpp, since in this case varid would be NULL.
This commit is contained in:
Pete Johns 2010-10-01 20:30:21 +10:00
parent 6eeed00888
commit c6acdccfa0
4 changed files with 115 additions and 14 deletions

View File

@ -3841,6 +3841,24 @@ void CheckOther::checkMathFunctions()
}
}
bool CheckOther::isIdentifierObjectType(const Token * const tok) const
{
const Token *decltok = Token::findmatch(_tokenizer->tokens(), "%varid%", tok->tokAt(1)->varId());
const std::string classDef = std::string("class|struct ") + std::string(decltok->previous()->strAt(0));
return Token::findmatch(_tokenizer->tokens(), classDef.c_str());
}
void CheckOther::CheckUnusedScopedObject()
{
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next())
{
if (Token::Match(tok, "[;{}] %var% (") && isIdentifierObjectType(tok))
{
misusedScopeObjectError(tok, tok->tokAt(1)->str());
}
}
}
void CheckOther::postIncrement()
@ -3862,13 +3880,11 @@ void CheckOther::postIncrement()
// Take a look at the variable declaration
const Token *decltok = Token::findmatch(_tokenizer->tokens(), "%varid%", tok2->tokAt(1)->varId());
const std::string classDef = std::string("class ") + std::string(decltok->previous()->strAt(0));
// Is the variable an iterator?
if (decltok && Token::Match(decltok->previous(), "iterator|const_iterator"))
postIncrementError(tok2, tok2->strAt(1), (std::string("++") == tok2->strAt(2)));
// Is the variable a class?
else if (Token::findmatch(_tokenizer->tokens(), classDef.c_str()))
else if (isIdentifierObjectType(tok2))
postIncrementError(tok2, tok2->strAt(1), (std::string("++") == tok2->strAt(2)));
}
}
@ -4091,3 +4107,8 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
"selfAssignment", "Redundant assignment of \"" + varname + "\" to itself");
}
void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& varname)
{
reportError(tok, Severity::error,
"unusedScopedObject", "instance of \"" + varname + "\" object destroyed immediately");
}

View File

@ -87,6 +87,7 @@ public:
// New type of check: Check execution paths
checkOther.executionPaths();
checkOther.CheckUnusedScopedObject();
}
@ -183,6 +184,9 @@ public:
/** @brief %Check for assigning a variable to itself*/
void checkSelfAssignment();
/** @brief %Check for objects that are destroyed immediately */
void CheckUnusedScopedObject();
// Error messages..
void cstyleCastError(const Token *tok);
void dangerousUsageStrtolError(const Token *tok);
@ -209,6 +213,7 @@ public:
void fflushOnInputStreamError(const Token *tok, const std::string &varname);
void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname);
void selfAssignmentError(const Token *tok, const std::string &varname);
void misusedScopeObjectError(const Token *tok, const std::string &varname);
void getErrorMessages()
{
@ -222,6 +227,7 @@ public:
zerodivError(0);
mathfunctionCallError(0);
fflushOnInputStreamError(0, "stdin");
misusedScopeObjectError(NULL, "varname");
// style
cstyleCastError(0);
@ -337,6 +343,14 @@ private:
return varname;
}
/**
* @brief query type of identifier
* @param tok Token of the identifier
* @return true if the identifier is of type 'class' or 'struct',
* false otherwise.
*/
bool isIdentifierObjectType(const Token* const tok) const;
};
/// @}
//---------------------------------------------------------------------------

View File

@ -417,16 +417,6 @@ bool Token::Match(const Token *tok, const char pattern[], unsigned int varid)
else if (firstWordEquals(p, "%varid%") == 0)
{
if (varid == 0)
{
std::list<ErrorLogger::ErrorMessage::FileLocation> locationList;
const ErrorLogger::ErrorMessage errmsg(locationList,
Severity::error,
"Internal error. Token::Match called with varid 0.",
"cppcheckError");
Check::reportError(errmsg);
}
if (tok->varId() != varid)
return false;

View File

@ -104,6 +104,10 @@ private:
TEST_CASE(selfAssignment);
TEST_CASE(testScanf1);
TEST_CASE(testScanf2);
TEST_CASE(trac1132);
TEST_CASE(testMisusedScopeObjectDoesNotPickFunction);
TEST_CASE(testMisusedScopeObjectPicksClass);
TEST_CASE(testMisusedScopeObjectPicksStruct);
}
void check(const char code[])
@ -134,6 +138,7 @@ private:
checkOther.checkFflushOnInputStream();
checkOther.checkSelfAssignment();
checkOther.invalidScanf();
checkOther.CheckUnusedScopedObject();
}
@ -2997,7 +3002,78 @@ private:
ASSERT_EQUALS("[test.cpp:6]: (style) scanf without field width limits can crash with huge input data\n"
"[test.cpp:7]: (style) scanf without field width limits can crash with huge input data\n", errout.str());
}
void trac1132()
{
errout.str("");
std::istringstream code("#include <iostream>\n"
"class Lock\n"
"{\n"
"public:\n"
" Lock(int i)\n"
" {\n"
" std::cout << \"Lock \" << i << std::endl;\n"
" }\n"
" ~Lock()\n"
" {\n"
" std::cout << \"~Lock\" << std::endl;\n"
" }\n"
"};\n"
"int main()\n"
"{\n"
" Lock(123);\n"
" std::cout << \"hello\" << std::endl;\n"
" return 0;\n"
"}\n"
);
Tokenizer tokenizer;
tokenizer.tokenize(code, "trac1132.cpp");
tokenizer.simplifyTokenList();
Settings settings;
CheckOther checkOther(&tokenizer, &settings, this);
checkOther.CheckUnusedScopedObject();
ASSERT_EQUALS("[trac1132.cpp:15]: (error) instance of \"Lock\" object destroyed immediately\n", errout.str());
}
void testMisusedScopeObjectDoesNotPickFunction()
{
check("int main()\n"
"{\n"
" CouldBeFunction(123);\n"
" return 0;\n"
"}\n"
);
ASSERT_EQUALS("", errout.str());
}
void testMisusedScopeObjectPicksClass()
{
check("class NotAFunction;\n"
"int function()\n"
"{\n"
" NotAFunction(123);\n"
" return 0;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:3]: (error) instance of \"NotAFunction\" object destroyed immediately\n", errout.str());
}
void testMisusedScopeObjectPicksStruct()
{
check("struct NotAClass;\n"
"bool func()\n"
"{\n"
" NotAClass(123);\n"
" return true;\n"
"}\n"
);
ASSERT_EQUALS("[test.cpp:3]: (error) instance of \"NotAClass\" object destroyed immediately\n", errout.str());
}
};
REGISTER_TEST(TestOther)