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

Refactorization: 

- Added missing separating comments between checks in checkother.cpp
- Moved checks related to strings into own file
This commit is contained in:
PKEuS 2014-08-27 09:42:09 +02:00
parent 0a738973cb
commit eba8c6f6c5
11 changed files with 1035 additions and 801 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

334
lib/checkother.cpp
View File

@ -273,6 +273,7 @@ void CheckOther::checkCastIntToCharAndBackError(const Token *tok, const std::str
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// Clarify calculation precedence for ternary operators.
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::clarifyCalculation() void CheckOther::clarifyCalculation()
{ {
@ -458,7 +459,9 @@ void CheckOther::clarifyStatementError(const Token *tok)
"Thus, the dereference is meaningless. Did you intend to write '(*A)++;'?"); "Thus, the dereference is meaningless. Did you intend to write '(*A)++;'?");
} }
//---------------------------------------------------------------------------
// Check for suspicious occurences of 'if(); {}'.
//---------------------------------------------------------------------------
void CheckOther::checkSuspiciousSemicolon() void CheckOther::checkSuspiciousSemicolon()
{ {
if (!_settings->inconclusive || !_settings->isEnabled("warning")) if (!_settings->inconclusive || !_settings->isEnabled("warning"))
@ -488,6 +491,7 @@ void CheckOther::SuspiciousSemicolonError(const Token* tok)
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// For C++ code, warn if C-style casts are used on pointer types
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::warningOldStylePointerCast() void CheckOther::warningOldStylePointerCast()
{ {
@ -1001,6 +1005,7 @@ void CheckOther::redundantBitwiseOperationInSwitchError(const Token *tok, const
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// Detect fall through cases (experimental).
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::checkSwitchCaseFallThrough() void CheckOther::checkSwitchCaseFallThrough()
{ {
@ -1456,49 +1461,6 @@ void CheckOther::invalidFunctionUsage()
} }
} }
} }
// sprintf|snprintf overlapping data
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
// Get variable id of target buffer..
unsigned int varid = 0;
if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% ,"))
varid = tok->tokAt(2)->varId();
else if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% . %var% ,"))
varid = tok->tokAt(4)->varId();
if (varid == 0)
continue;
// goto next argument
const Token *tok2 = tok->tokAt(2)->nextArgument();
if (tok->str() == "snprintf" || tok->str() == "swprintf") { // Jump over second parameter for snprintf and swprintf
tok2 = tok2->nextArgument();
if (!tok2)
continue;
}
// is any source buffer overlapping the target buffer?
do {
if (Token::Match(tok2, "%varid% [,)]", varid)) {
sprintfOverlappingDataError(tok2, tok2->str());
break;
}
} while (nullptr != (tok2 = tok2->nextArgument()));
}
}
void CheckOther::sprintfOverlappingDataError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::error, "sprintfOverlappingData",
"Undefined behavior: Variable '" + varname + "' is used as parameter and destination in s[n]printf().\n"
"The variable '" + varname + "' is used both as a parameter and as destination in "
"s[n]printf(). The origin and destination buffers overlap. Quote from glibc (C-library) "
"documentation (http://www.gnu.org/software/libc/manual/html_mono/libc.html#Formatted-Output-Functions): "
"\"If copying takes place between objects that overlap as a result of a call "
"to sprintf() or snprintf(), the results are undefined.\"");
} }
void CheckOther::invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr) void CheckOther::invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr)
@ -1821,8 +1783,10 @@ void CheckOther::checkVariableScope()
elseif = true; elseif = true;
else if (Token::simpleMatch(endif, "} else {") && Token::simpleMatch(endif->linkAt(2),"} }")) else if (Token::simpleMatch(endif, "} else {") && Token::simpleMatch(endif->linkAt(2),"} }"))
elseif = true; elseif = true;
if (elseif && Token::findmatch(tok->next(), "%varid%", tok->linkAt(1), var->declarationId())) if (elseif && Token::findmatch(tok->next(), "%varid%", tok->linkAt(1), var->declarationId())) {
reduce = false; reduce = false;
break;
}
} else if (tok->varId() == var->declarationId() || tok->str() == "goto") { } else if (tok->varId() == var->declarationId() || tok->str() == "goto") {
reduce = false; reduce = false;
break; break;
@ -1940,6 +1904,9 @@ void CheckOther::variableScopeError(const Token *tok, const std::string &varname
"When you see this message it is always safe to reduce the variable scope 1 level."); "When you see this message it is always safe to reduce the variable scope 1 level.");
} }
//---------------------------------------------------------------------------
// Comma in return statement: return a+1, b++;. (experimental)
//---------------------------------------------------------------------------
void CheckOther::checkCommaSeparatedReturn() void CheckOther::checkCommaSeparatedReturn()
{ {
// This is experimental for now. See #5076 // This is experimental for now. See #5076
@ -2202,34 +2169,7 @@ void CheckOther::constStatementError(const Token *tok, const std::string &type)
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// str plus char // Detect division by zero.
//---------------------------------------------------------------------------
void CheckOther::strPlusChar()
{
//_tokenizer->tokens()->printAst(true);
//_tokenizer->tokens()->printOut();
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->str() == "+") {
if (tok->astOperand1() && (tok->astOperand1()->type() == Token::eString)) { // string literal...
if (tok->astOperand2() && (tok->astOperand2()->type() == Token::eChar || isChar(tok->astOperand2()->variable()))) // added to char variable or char constant
strPlusCharError(tok);
}
}
}
}
}
void CheckOther::strPlusCharError(const Token *tok)
{
reportError(tok, Severity::error, "strPlusChar", "Unusual pointer arithmetic. A value of type 'char' is added to a string literal.");
}
//---------------------------------------------------------------------------
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::checkZeroDivision() void CheckOther::checkZeroDivision()
{ {
@ -2289,12 +2229,12 @@ void CheckOther::zerodivcondError(const Token *tokcond, const Token *tokdiv, boo
const std::string linenr(MathLib::toString(tokdiv ? tokdiv->linenr() : 0)); const std::string linenr(MathLib::toString(tokdiv ? tokdiv->linenr() : 0));
reportError(callstack, Severity::warning, "zerodivcond", "Either the condition '"+condition+"' is useless or there is division by zero at line " + linenr + ".", inconclusive); reportError(callstack, Severity::warning, "zerodivcond", "Either the condition '"+condition+"' is useless or there is division by zero at line " + linenr + ".", inconclusive);
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// Check for NaN (not-a-number) in an arithmetic expression, e.g.
// double d = 1.0 / 0.0 + 100.0;
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
/** @brief Check for NaN (not-a-number) in an arithmetic expression
* @note e.g. double d = 1.0 / 0.0 + 100.0;
*/
void CheckOther::checkNanInArithmeticExpression() void CheckOther::checkNanInArithmeticExpression()
{ {
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
@ -2315,6 +2255,7 @@ void CheckOther::nanInArithmeticExpressionError(const Token *tok)
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// Detect passing wrong values to <cmath> functions like atan(0, x);
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::checkMathFunctions() void CheckOther::checkMathFunctions()
{ {
@ -2383,7 +2324,9 @@ void CheckOther::mathfunctionCallError(const Token *tok, const unsigned int numP
} else } else
reportError(tok, Severity::error, "wrongmathcall", "Passing value '#' to #() leads to undefined result."); reportError(tok, Severity::error, "wrongmathcall", "Passing value '#' to #() leads to undefined result.");
} }
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
// Creating instance of clases which are destroyed immediately
//--------------------------------------------------------------------------- //---------------------------------------------------------------------------
void CheckOther::checkMisusedScopedObject() void CheckOther::checkMisusedScopedObject()
{ {
@ -2416,67 +2359,6 @@ void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& va
"unusedScopedObject", "Instance of '" + varname + "' object is destroyed immediately."); "unusedScopedObject", "Instance of '" + varname + "' object is destroyed immediately.");
} }
//---------------------------------------------------------------------------
//---------------------------------------------------------------------------
void CheckOther::checkIncorrectStringCompare()
{
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()) {
// skip "assert(str && ..)" and "assert(.. && str)"
if (Token::Match(tok, "%var% (") &&
(Token::Match(tok->tokAt(2), "%str% &&") || Token::Match(tok->next()->link()->tokAt(-2), "&& %str% )")) &&
(tok->str().find("assert")+6U==tok->str().size() || tok->str().find("ASSERT")+6U==tok->str().size()))
tok = tok->next()->link();
if (Token::simpleMatch(tok, ". substr (") && Token::Match(tok->tokAt(3)->nextArgument(), "%num% )")) {
MathLib::bigint clen = MathLib::toLongNumber(tok->linkAt(2)->strAt(-1));
const Token* begin = tok->previous();
for (;;) { // Find start of statement
while (begin->link() && Token::Match(begin, "]|)|>"))
begin = begin->link()->previous();
if (Token::Match(begin->previous(), ".|::"))
begin = begin->tokAt(-2);
else
break;
}
begin = begin->previous();
const Token* end = tok->linkAt(2)->next();
if (Token::Match(begin->previous(), "%str% ==|!=") && begin->strAt(-2) != "+") {
std::size_t slen = Token::getStrLength(begin->previous());
if (clen != (int)slen) {
incorrectStringCompareError(tok->next(), "substr", begin->strAt(-1));
}
} else if (Token::Match(end, "==|!= %str% !!+")) {
std::size_t slen = Token::getStrLength(end->next());
if (clen != (int)slen) {
incorrectStringCompareError(tok->next(), "substr", end->strAt(1));
}
}
} else if (Token::Match(tok, "&&|%oror%|( %str% &&|%oror%|)") && !Token::Match(tok, "( %str% )")) {
incorrectStringBooleanError(tok->next(), tok->strAt(1));
} else if (Token::Match(tok, "if|while ( %str% )")) {
incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2));
}
}
}
}
void CheckOther::incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string)
{
reportError(tok, Severity::warning, "incorrectStringCompare", "String literal " + string + " doesn't match length argument for " + func + "().");
}
void CheckOther::incorrectStringBooleanError(const Token *tok, const std::string& string)
{
reportError(tok, Severity::warning, "incorrectStringBooleanError", "Conversion of string literal " + string + " to bool always evaluates to true.");
}
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
// check for duplicate code in if and else branches // check for duplicate code in if and else branches
// if (a) { b = true; } else { b = true; } // if (a) { b = true; } else { b = true; }
@ -2722,6 +2604,12 @@ void CheckOther::doubleCloseDirError(const Token *tok, const std::string &varnam
reportError(tok, Severity::error, "doubleCloseDir", "Directory handle '" + varname +"' closed twice."); reportError(tok, Severity::error, "doubleCloseDir", "Directory handle '" + varname +"' closed twice.");
} }
//---------------------------------------------------------------------------
// check for the same expression on both sides of an operator
// (x == x), (x && x), (x || x)
// (x.y == x.y), (x.y && x.y), (x.y || x.y)
//---------------------------------------------------------------------------
namespace { namespace {
bool notconst(const Function* func) bool notconst(const Function* func)
{ {
@ -2752,12 +2640,6 @@ namespace {
} }
} }
//---------------------------------------------------------------------------
// check for the same expression on both sides of an operator
// (x == x), (x && x), (x || x)
// (x.y == x.y), (x.y && x.y), (x.y || x.y)
//---------------------------------------------------------------------------
static bool isWithoutSideEffects(const Tokenizer *tokenizer, const Token* tok) static bool isWithoutSideEffects(const Tokenizer *tokenizer, const Token* tok)
{ {
if (!tokenizer->isCPP()) if (!tokenizer->isCPP())
@ -2842,145 +2724,6 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
"selfAssignment", "Redundant assignment of '" + varname + "' to itself."); "selfAssignment", "Redundant assignment of '" + varname + "' to itself.");
} }
//---------------------------------------------------------------------------
// Check for string comparison involving two static strings.
// if(strcmp("00FF00","00FF00")==0) // <- statement is always true
//---------------------------------------------------------------------------
void CheckOther::checkAlwaysTrueOrFalseStringCompare()
{
if (!_settings->isEnabled("warning"))
return;
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "strncmp|strcmp|stricmp|strcmpi|strcasecmp|wcscmp|wcsncmp (")) {
if (Token::Match(tok->tokAt(2), "%str% , %str%")) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(4);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(5);
} else if (Token::Match(tok->tokAt(2), "%var% , %var%")) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(4);
if (str1 == str2)
alwaysTrueStringVariableCompareError(tok, str1, str2);
tok = tok->tokAt(5);
} else if (Token::Match(tok->tokAt(2), "%var% . c_str ( ) , %var% . c_str ( )")) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(8);
if (str1 == str2)
alwaysTrueStringVariableCompareError(tok, str1, str2);
tok = tok->tokAt(13);
}
} else if (Token::Match(tok, "QString :: compare ( %str% , %str% )")) {
const std::string &str1 = tok->strAt(4);
const std::string &str2 = tok->strAt(6);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(7);
} else if (Token::Match(tok, "!!+ %str% ==|!= %str% !!+")) {
const std::string &str1 = tok->strAt(1);
const std::string &str2 = tok->strAt(3);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(5);
}
}
}
void CheckOther::alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2)
{
const std::size_t stringLen = 10;
const std::string string1 = (str1.size() < stringLen) ? str1 : (str1.substr(0, stringLen-2) + "..");
const std::string string2 = (str2.size() < stringLen) ? str2 : (str2.substr(0, stringLen-2) + "..");
reportError(tok, Severity::warning, "staticStringCompare",
"Unnecessary comparison of static strings.\n"
"The compared strings, '" + string1 + "' and '" + string2 + "', are always " + (str1==str2?"identical":"unequal") + ". "
"Therefore the comparison is unnecessary and looks suspicious.");
}
void CheckOther::alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2)
{
reportError(tok, Severity::warning, "stringCompare",
"Comparison of identical string variables.\n"
"The compared strings, '" + str1 + "' and '" + str2 + "', are identical. "
"This could be a logic bug.");
}
//-----------------------------------------------------------------------------
//-----------------------------------------------------------------------------
void CheckOther::checkSuspiciousStringCompare()
{
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->type() != Token::eComparisonOp)
continue;
const Token* varTok = tok->astOperand1();
const Token* litTok = tok->astOperand2();
if (!varTok || !litTok) // <- failed to create AST for comparison
continue;
if (varTok->type() == Token::eString || varTok->type() == Token::eNumber)
std::swap(varTok, litTok);
else if (litTok->type() != Token::eString && litTok->type() != Token::eNumber)
continue;
// Pointer addition?
if (varTok->str() == "+" && _tokenizer->isC()) {
const Token *tokens[2] = { varTok->astOperand1(), varTok->astOperand2() };
for (int nr = 0; nr < 2; nr++) {
const Token *t = tokens[nr];
while (t && (t->str() == "." || t->str() == "::"))
t = t->astOperand2();
if (t && t->variable() && t->variable()->isPointer())
varTok = t;
}
}
if (varTok->str() == "*") {
if (!_tokenizer->isC() || varTok->astOperand2() != nullptr || litTok->type() != Token::eString)
continue;
varTok = varTok->astOperand1();
}
while (varTok && (varTok->str() == "." || varTok->str() == "::"))
varTok = varTok->astOperand2();
if (!varTok || !varTok->isName())
continue;
const Variable *var = varTok->variable();
while (Token::Match(varTok->astParent(), "[.*]"))
varTok = varTok->astParent();
const std::string varname = varTok->expressionString();
if (litTok->type() == Token::eString) {
if (_tokenizer->isC() || (var && var->isArrayOrPointer()))
suspiciousStringCompareError(tok, varname);
} else if (litTok->originalName() == "'\\0'" && var && var->isPointer()) {
suspiciousStringCompareError_char(tok, varname);
}
}
}
}
void CheckOther::suspiciousStringCompareError(const Token* tok, const std::string& var)
{
reportError(tok, Severity::warning, "literalWithCharPtrCompare",
"String literal compared with variable '" + var + "'. Did you intend to use strcmp() instead?");
}
void CheckOther::suspiciousStringCompareError_char(const Token* tok, const std::string& var)
{
reportError(tok, Severity::warning, "charLiteralWithCharPtrCompare",
"Char literal compared with pointer '" + var + "'. Did you intend to dereference it?");
}
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
// Check is a comparison of two variables leads to condition, which is // Check is a comparison of two variables leads to condition, which is
// always true or false. // always true or false.
@ -3010,10 +2753,10 @@ void CheckOther::checkComparisonFunctionIsAlwaysTrueOrFalse()
const std::string& varNameLeft = tok->strAt(2); // get the left variable name const std::string& varNameLeft = tok->strAt(2); // get the left variable name
if (functionName == "isgreater" || functionName == "isless" || functionName == "islessgreater") { if (functionName == "isgreater" || functionName == "isless" || functionName == "islessgreater") {
// e.g.: isgreater(x,x) --> (x)>(x) --> false // e.g.: isgreater(x,x) --> (x)>(x) --> false
checkComparisonFunctionIsAlwaysTrueOrFalseError(tok,functionName,varNameLeft,false); checkComparisonFunctionIsAlwaysTrueOrFalseError(tok, functionName, varNameLeft, false);
} else { // functionName == "isgreaterequal" || functionName == "islessequal" } else { // functionName == "isgreaterequal" || functionName == "islessequal"
// e.g.: isgreaterequal(x,x) --> (x)>=(x) --> true // e.g.: isgreaterequal(x,x) --> (x)>=(x) --> true
checkComparisonFunctionIsAlwaysTrueOrFalseError(tok,functionName,varNameLeft,true); checkComparisonFunctionIsAlwaysTrueOrFalseError(tok, functionName, varNameLeft, true);
} }
} }
} }
@ -3024,12 +2767,13 @@ void CheckOther::checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* to
{ {
const std::string strResult = result ? "true" : "false"; const std::string strResult = result ? "true" : "false";
reportError(tok, Severity::warning, "comparisonFunctionIsAlwaysTrueOrFalse", reportError(tok, Severity::warning, "comparisonFunctionIsAlwaysTrueOrFalse",
"Comparison of two identical variables with "+functionName+"("+varName+","+varName+") evaluates always to "+strResult+".\n" "Comparison of two identical variables with " + functionName + "(" + varName + "," + varName + ") evaluates always to " + strResult + ".\n"
"The function "+functionName+" is designed to compare two variables. Calling this function with one variable ("+varName+") " "The function " + functionName + " is designed to compare two variables. Calling this function with one variable (" + varName + ") "
"for both parameters leads to a statement which is always "+strResult+"."); "for both parameters leads to a statement which is always " + strResult + ".");
} }
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
// Detect "(var % val1) > val2" where val2 is >= val1.
//----------------------------------------------------------------------------- //-----------------------------------------------------------------------------
void CheckOther::checkModuloAlwaysTrueFalse() void CheckOther::checkModuloAlwaysTrueFalse()
{ {
@ -3211,10 +2955,10 @@ static bool constructorTakesReference(const Scope * const classScope)
return false; return false;
} }
/* //---------------------------------------------------------------------------
This check rule works for checking the "const A a = getA()" usage when getA() returns "const A &" or "A &". // This check rule works for checking the "const A a = getA()" usage when getA() returns "const A &" or "A &".
In most scenarios, "const A & a = getA()" will be more efficient. // In most scenarios, "const A & a = getA()" will be more efficient.
*/ //---------------------------------------------------------------------------
void CheckOther::checkRedundantCopy() void CheckOther::checkRedundantCopy()
{ {
if (!_settings->isEnabled("performance") || _tokenizer->isC()) if (!_settings->isEnabled("performance") || _tokenizer->isC())
@ -3354,6 +3098,10 @@ void CheckOther::incompleteArrayFillError(const Token* tok, const std::string& b
} }
//---------------------------------------------------------------------------
// Detect oppositing inner and outer conditions
//---------------------------------------------------------------------------
void CheckOther::oppositeInnerCondition() void CheckOther::oppositeInnerCondition()
{ {
if (!_settings->isEnabled("warning")) if (!_settings->isEnabled("warning"))
@ -3450,6 +3198,10 @@ void CheckOther::oppositeInnerConditionError(const Token *tok1, const Token* tok
} }
//---------------------------------------------------------------------------
// Detect NULL being passed to variadic function.
//---------------------------------------------------------------------------
void CheckOther::checkVarFuncNullUB() void CheckOther::checkVarFuncNullUB()
{ {
if (!_settings->isEnabled("portability")) if (!_settings->isEnabled("portability"))

34
lib/checkother.h
View File

@ -58,7 +58,6 @@ public:
checkOther.invalidPointerCast(); checkOther.invalidPointerCast();
checkOther.checkUnsignedDivision(); checkOther.checkUnsignedDivision();
checkOther.checkCharVariable(); checkOther.checkCharVariable();
checkOther.strPlusChar();
checkOther.checkRedundantAssignment(); checkOther.checkRedundantAssignment();
checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkRedundantAssignmentInSwitch();
checkOther.checkSuspiciousCaseInSwitch(); checkOther.checkSuspiciousCaseInSwitch();
@ -70,7 +69,6 @@ public:
checkOther.clarifyCondition(); // not simplified because ifAssign checkOther.clarifyCondition(); // not simplified because ifAssign
checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574) checkOther.checkSignOfUnsignedVariable(); // don't ignore casts (#3574)
checkOther.checkIncompleteArrayFill(); checkOther.checkIncompleteArrayFill();
checkOther.checkSuspiciousStringCompare();
checkOther.checkVarFuncNullUB(); checkOther.checkVarFuncNullUB();
checkOther.checkNanInArithmeticExpression(); checkOther.checkNanInArithmeticExpression();
checkOther.checkCommaSeparatedReturn(); checkOther.checkCommaSeparatedReturn();
@ -97,9 +95,7 @@ public:
checkOther.checkMisusedScopedObject(); checkOther.checkMisusedScopedObject();
checkOther.checkMemsetZeroBytes(); checkOther.checkMemsetZeroBytes();
checkOther.checkMemsetInvalid2ndParam(); checkOther.checkMemsetInvalid2ndParam();
checkOther.checkIncorrectStringCompare();
checkOther.checkSwitchCaseFallThrough(); checkOther.checkSwitchCaseFallThrough();
checkOther.checkAlwaysTrueOrFalseStringCompare();
checkOther.checkModuloAlwaysTrueFalse(); checkOther.checkModuloAlwaysTrueFalse();
checkOther.checkPipeParameterSize(); checkOther.checkPipeParameterSize();
@ -158,9 +154,6 @@ public:
/** @brief Incomplete statement. A statement that only contains a constant or variable */ /** @brief Incomplete statement. A statement that only contains a constant or variable */
void checkIncompleteStatement(); void checkIncompleteStatement();
/** @brief str plus char (unusual pointer arithmetic) */
void strPlusChar();
/** @brief %Check zero division*/ /** @brief %Check zero division*/
void checkZeroDivision(); void checkZeroDivision();
@ -203,12 +196,6 @@ public:
/** @brief %Check for invalid 2nd parameter of memset() */ /** @brief %Check for invalid 2nd parameter of memset() */
void checkMemsetInvalid2ndParam(); void checkMemsetInvalid2ndParam();
/** @brief %Check for using bad usage of strncmp and substr */
void checkIncorrectStringCompare();
/** @brief %Check for comparison of a string literal with a char* variable */
void checkSuspiciousStringCompare();
/** @brief %Check for suspicious code where multiple if have the same expression (e.g "if (a) { } else if (a) { }") */ /** @brief %Check for suspicious code where multiple if have the same expression (e.g "if (a) { } else if (a) { }") */
void checkDuplicateIf(); void checkDuplicateIf();
@ -218,9 +205,6 @@ public:
/** @brief %Check for suspicious code with the same expression on both sides of operator (e.g "if (a && a)") */ /** @brief %Check for suspicious code with the same expression on both sides of operator (e.g "if (a && a)") */
void checkDuplicateExpression(); void checkDuplicateExpression();
/** @brief %Check for suspicious code that compares string literals for equality */
void checkAlwaysTrueOrFalseStringCompare();
/** @brief %Check for suspicious usage of modulo (e.g. "if(var % 4 == 4)") */ /** @brief %Check for suspicious usage of modulo (e.g. "if(var % 4 == 4)") */
void checkModuloAlwaysTrueFalse(); void checkModuloAlwaysTrueFalse();
@ -277,7 +261,6 @@ private:
void redundantGetAndSetUserIdError(const Token *tok); void redundantGetAndSetUserIdError(const Token *tok);
void cstyleCastError(const Token *tok); void cstyleCastError(const Token *tok);
void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive);
void sprintfOverlappingDataError(const Token *tok, const std::string &varname);
void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr); void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const std::string &validstr);
void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr); void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr);
void udivError(const Token *tok, bool inconclusive); void udivError(const Token *tok, bool inconclusive);
@ -286,7 +269,6 @@ private:
void charArrayIndexError(const Token *tok); void charArrayIndexError(const Token *tok);
void charBitOpError(const Token *tok); void charBitOpError(const Token *tok);
void variableScopeError(const Token *tok, const std::string &varname); void variableScopeError(const Token *tok, const std::string &varname);
void strPlusCharError(const Token *tok);
void zerodivError(const Token *tok, bool inconclusive); void zerodivError(const Token *tok, bool inconclusive);
void zerodivcondError(const Token *tokcond, const Token *tokdiv, bool inconclusive); void zerodivcondError(const Token *tokcond, const Token *tokdiv, bool inconclusive);
void nanInArithmeticExpressionError(const Token *tok); void nanInArithmeticExpressionError(const Token *tok);
@ -306,15 +288,11 @@ private:
void memsetZeroBytesError(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); void memsetFloatError(const Token *tok, const std::string &var_value);
void memsetValueOutOfRangeError(const Token *tok, const std::string &value); void memsetValueOutOfRangeError(const Token *tok, const std::string &value);
void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string);
void incorrectStringBooleanError(const Token *tok, const std::string& string);
void duplicateIfError(const Token *tok1, const Token *tok2); void duplicateIfError(const Token *tok1, const Token *tok2);
void duplicateBranchError(const Token *tok1, const Token *tok2); void duplicateBranchError(const Token *tok1, const Token *tok2);
void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op); void duplicateExpressionError(const Token *tok1, const Token *tok2, const std::string &op);
void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2); void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2); void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void suspiciousStringCompareError(const Token* tok, const std::string& var);
void suspiciousStringCompareError_char(const Token* tok, const std::string& var);
void duplicateBreakError(const Token *tok, bool inconclusive); void duplicateBreakError(const Token *tok, bool inconclusive);
void unreachableCodeError(const Token* tok, bool inconclusive); void unreachableCodeError(const Token* tok, bool inconclusive);
void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive); void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive);
@ -334,7 +312,6 @@ private:
CheckOther c(0, settings, errorLogger); CheckOther c(0, settings, errorLogger);
// error // error
c.sprintfOverlappingDataError(0, "varname");
c.invalidFunctionArgError(0, "func_name", 1, "1-4"); c.invalidFunctionArgError(0, "func_name", 1, "1-4");
c.invalidFunctionArgBoolError(0, "func_name", 1); c.invalidFunctionArgBoolError(0, "func_name", 1);
c.udivError(0, false); c.udivError(0, false);
@ -362,7 +339,6 @@ private:
c.charArrayIndexError(0); c.charArrayIndexError(0);
c.charBitOpError(0); c.charBitOpError(0);
c.variableScopeError(0, "varname"); c.variableScopeError(0, "varname");
c.strPlusCharError(0);
c.redundantAssignmentInSwitchError(0, 0, "var"); c.redundantAssignmentInSwitchError(0, 0, "var");
c.redundantCopyInSwitchError(0, 0, "var"); c.redundantCopyInSwitchError(0, 0, "var");
c.switchCaseFallThrough(0); c.switchCaseFallThrough(0);
@ -377,14 +353,8 @@ private:
c.clarifyCalculationError(0, "+"); c.clarifyCalculationError(0, "+");
c.clarifyConditionError(0, true, false); c.clarifyConditionError(0, true, false);
c.clarifyStatementError(0); c.clarifyStatementError(0);
c.incorrectStringCompareError(0, "substr", "\"Hello World\"");
c.suspiciousStringCompareError(0, "foo");
c.suspiciousStringCompareError_char(0, "foo");
c.incorrectStringBooleanError(0, "\"Hello World\"");
c.duplicateBranchError(0, 0); c.duplicateBranchError(0, 0);
c.duplicateExpressionError(0, 0, "&&"); c.duplicateExpressionError(0, 0, "&&");
c.alwaysTrueFalseStringCompareError(0, "str1", "str2");
c.alwaysTrueStringVariableCompareError(0, "varname1", "varname2");
c.duplicateBreakError(0, false); c.duplicateBreakError(0, false);
c.unreachableCodeError(0, false); c.unreachableCodeError(0, false);
c.unsignedLessThanZeroError(0, "varname", false); c.unsignedLessThanZeroError(0, "varname", false);
@ -411,7 +381,6 @@ private:
"* division with zero\n" "* division with zero\n"
"* scoped object destroyed immediately after construction\n" "* scoped object destroyed immediately after construction\n"
"* assignment in an assert statement\n" "* assignment in an assert statement\n"
"* incorrect length arguments for 'substr' and 'strncmp'\n"
"* free() or delete of an invalid memory location\n" "* free() or delete of an invalid memory location\n"
"* double free() or double closedir()\n" "* double free() or double closedir()\n"
"* bitwise operation with negative right operand\n" "* bitwise operation with negative right operand\n"
@ -453,9 +422,6 @@ private:
"* Comparison of values leading always to true or false\n" "* Comparison of values leading always to true or false\n"
"* Clarify calculation with parentheses\n" "* Clarify calculation with parentheses\n"
"* suspicious condition (assignment+comparison)\n" "* suspicious condition (assignment+comparison)\n"
"* suspicious condition (runtime comparison of string literals)\n"
"* suspicious condition (string literals as boolean)\n"
"* suspicious comparison of a string literal with a char* variable\n"
"* suspicious comparison of '\\0' with a char* variable\n" "* suspicious comparison of '\\0' with a char* variable\n"
"* duplicate break statement\n" "* duplicate break statement\n"
"* unreachable code\n" "* unreachable code\n"

314
lib/checkstring.cpp Normal file
View File

@ -0,0 +1,314 @@
/*
* 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 "checkstring.h"
#include "symboldatabase.h"
//---------------------------------------------------------------------------
// Register this check class (by creating a static instance of it)
namespace {
CheckString instance;
}
//---------------------------------------------------------------------------
// Check for string comparison involving two static strings.
// if(strcmp("00FF00","00FF00")==0) // <- statement is always true
//---------------------------------------------------------------------------
void CheckString::checkAlwaysTrueOrFalseStringCompare()
{
if (!_settings->isEnabled("warning"))
return;
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "strncmp|strcmp|stricmp|strcmpi|strcasecmp|wcscmp|wcsncmp (")) {
if (Token::Match(tok->tokAt(2), "%str% , %str%")) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(4);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(5);
} else if (Token::Match(tok->tokAt(2), "%var% , %var%")) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(4);
if (str1 == str2)
alwaysTrueStringVariableCompareError(tok, str1, str2);
tok = tok->tokAt(5);
} else if (Token::Match(tok->tokAt(2), "%var% . c_str ( ) , %var% . c_str ( )")) {
const std::string &str1 = tok->strAt(2);
const std::string &str2 = tok->strAt(8);
if (str1 == str2)
alwaysTrueStringVariableCompareError(tok, str1, str2);
tok = tok->tokAt(13);
}
} else if (Token::Match(tok, "QString :: compare ( %str% , %str% )")) {
const std::string &str1 = tok->strAt(4);
const std::string &str2 = tok->strAt(6);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(7);
} else if (Token::Match(tok, "!!+ %str% ==|!= %str% !!+")) {
const std::string &str1 = tok->strAt(1);
const std::string &str2 = tok->strAt(3);
alwaysTrueFalseStringCompareError(tok, str1, str2);
tok = tok->tokAt(5);
}
}
}
void CheckString::alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2)
{
const std::size_t stringLen = 10;
const std::string string1 = (str1.size() < stringLen) ? str1 : (str1.substr(0, stringLen-2) + "..");
const std::string string2 = (str2.size() < stringLen) ? str2 : (str2.substr(0, stringLen-2) + "..");
reportError(tok, Severity::warning, "staticStringCompare",
"Unnecessary comparison of static strings.\n"
"The compared strings, '" + string1 + "' and '" + string2 + "', are always " + (str1==str2?"identical":"unequal") + ". "
"Therefore the comparison is unnecessary and looks suspicious.");
}
void CheckString::alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2)
{
reportError(tok, Severity::warning, "stringCompare",
"Comparison of identical string variables.\n"
"The compared strings, '" + str1 + "' and '" + str2 + "', are identical. "
"This could be a logic bug.");
}
//-----------------------------------------------------------------------------
// Detect "str == '\0'" where "*str == '\0'" is correct.
// Comparing char* with each other instead of using strcmp()
//-----------------------------------------------------------------------------
void CheckString::checkSuspiciousStringCompare()
{
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->type() != Token::eComparisonOp)
continue;
const Token* varTok = tok->astOperand1();
const Token* litTok = tok->astOperand2();
if (!varTok || !litTok) // <- failed to create AST for comparison
continue;
if (varTok->type() == Token::eString || varTok->type() == Token::eNumber)
std::swap(varTok, litTok);
else if (litTok->type() != Token::eString && litTok->type() != Token::eNumber)
continue;
// Pointer addition?
if (varTok->str() == "+" && _tokenizer->isC()) {
const Token *tokens[2] = { varTok->astOperand1(), varTok->astOperand2() };
for (int nr = 0; nr < 2; nr++) {
const Token *t = tokens[nr];
while (t && (t->str() == "." || t->str() == "::"))
t = t->astOperand2();
if (t && t->variable() && t->variable()->isPointer())
varTok = t;
}
}
if (varTok->str() == "*") {
if (!_tokenizer->isC() || varTok->astOperand2() != nullptr || litTok->type() != Token::eString)
continue;
varTok = varTok->astOperand1();
}
while (varTok && (varTok->str() == "." || varTok->str() == "::"))
varTok = varTok->astOperand2();
if (!varTok || !varTok->isName())
continue;
const Variable *var = varTok->variable();
while (Token::Match(varTok->astParent(), "[.*]"))
varTok = varTok->astParent();
const std::string varname = varTok->expressionString();
if (litTok->type() == Token::eString) {
if (_tokenizer->isC() || (var && var->isArrayOrPointer()))
suspiciousStringCompareError(tok, varname);
} else if (litTok->originalName() == "'\\0'" && var && var->isPointer()) {
suspiciousStringCompareError_char(tok, varname);
}
}
}
}
void CheckString::suspiciousStringCompareError(const Token* tok, const std::string& var)
{
reportError(tok, Severity::warning, "literalWithCharPtrCompare",
"String literal compared with variable '" + var + "'. Did you intend to use strcmp() instead?");
}
void CheckString::suspiciousStringCompareError_char(const Token* tok, const std::string& var)
{
reportError(tok, Severity::warning, "charLiteralWithCharPtrCompare",
"Char literal compared with pointer '" + var + "'. Did you intend to dereference it?");
}
//---------------------------------------------------------------------------
// Adding C-string and char with operator+
//---------------------------------------------------------------------------
static bool isChar(const Variable* var)
{
return (var && !var->isPointer() && !var->isArray() && var->typeStartToken()->str() == "char");
}
void CheckString::strPlusChar()
{
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->str() == "+") {
if (tok->astOperand1() && (tok->astOperand1()->type() == Token::eString)) { // string literal...
if (tok->astOperand2() && (tok->astOperand2()->type() == Token::eChar || isChar(tok->astOperand2()->variable()))) // added to char variable or char constant
strPlusCharError(tok);
}
}
}
}
}
void CheckString::strPlusCharError(const Token *tok)
{
reportError(tok, Severity::error, "strPlusChar", "Unusual pointer arithmetic. A value of type 'char' is added to a string literal.");
}
//---------------------------------------------------------------------------
// Implicit casts of string literals to bool
// Comparing string literal with strlen() with wrong length
//---------------------------------------------------------------------------
void CheckString::checkIncorrectStringCompare()
{
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()) {
// skip "assert(str && ..)" and "assert(.. && str)"
if (Token::Match(tok, "%var% (") &&
(Token::Match(tok->tokAt(2), "%str% &&") || Token::Match(tok->next()->link()->tokAt(-2), "&& %str% )")) &&
(tok->str().find("assert") + 6U == tok->str().size() || tok->str().find("ASSERT") + 6U == tok->str().size()))
tok = tok->next()->link();
if (Token::simpleMatch(tok, ". substr (") && Token::Match(tok->tokAt(3)->nextArgument(), "%num% )")) {
MathLib::bigint clen = MathLib::toLongNumber(tok->linkAt(2)->strAt(-1));
const Token* begin = tok->previous();
for (;;) { // Find start of statement
while (begin->link() && Token::Match(begin, "]|)|>"))
begin = begin->link()->previous();
if (Token::Match(begin->previous(), ".|::"))
begin = begin->tokAt(-2);
else
break;
}
begin = begin->previous();
const Token* end = tok->linkAt(2)->next();
if (Token::Match(begin->previous(), "%str% ==|!=") && begin->strAt(-2) != "+") {
std::size_t slen = Token::getStrLength(begin->previous());
if (clen != (int)slen) {
incorrectStringCompareError(tok->next(), "substr", begin->strAt(-1));
}
} else if (Token::Match(end, "==|!= %str% !!+")) {
std::size_t slen = Token::getStrLength(end->next());
if (clen != (int)slen) {
incorrectStringCompareError(tok->next(), "substr", end->strAt(1));
}
}
} else if (Token::Match(tok, "&&|%oror%|( %str% &&|%oror%|)") && !Token::Match(tok, "( %str% )")) {
incorrectStringBooleanError(tok->next(), tok->strAt(1));
} else if (Token::Match(tok, "if|while ( %str% )")) {
incorrectStringBooleanError(tok->tokAt(2), tok->strAt(2));
}
}
}
}
void CheckString::incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string)
{
reportError(tok, Severity::warning, "incorrectStringCompare", "String literal " + string + " doesn't match length argument for " + func + "().");
}
void CheckString::incorrectStringBooleanError(const Token *tok, const std::string& string)
{
reportError(tok, Severity::warning, "incorrectStringBooleanError", "Conversion of string literal " + string + " to bool always evaluates to true.");
}
//---------------------------------------------------------------------------
// Overlapping source and destination passed to sprintf().
//---------------------------------------------------------------------------
void CheckString::sprintfOverlappingData()
{
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
// Get variable id of target buffer..
unsigned int varid = 0;
if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% ,"))
varid = tok->tokAt(2)->varId();
else if (Token::Match(tok, "sprintf|snprintf|swprintf ( %var% . %var% ,"))
varid = tok->tokAt(4)->varId();
if (varid == 0)
continue;
// goto next argument
const Token *tok2 = tok->tokAt(2)->nextArgument();
if (tok->str() == "snprintf" || tok->str() == "swprintf") { // Jump over second parameter for snprintf and swprintf
tok2 = tok2->nextArgument();
if (!tok2)
continue;
}
// is any source buffer overlapping the target buffer?
do {
if (Token::Match(tok2, "%varid% [,)]", varid)) {
sprintfOverlappingDataError(tok2, tok2->str());
break;
}
} while (nullptr != (tok2 = tok2->nextArgument()));
}
}
void CheckString::sprintfOverlappingDataError(const Token *tok, const std::string &varname)
{
reportError(tok, Severity::error, "sprintfOverlappingData",
"Undefined behavior: Variable '" + varname + "' is used as parameter and destination in s[n]printf().\n"
"The variable '" + varname + "' is used both as a parameter and as destination in "
"s[n]printf(). The origin and destination buffers overlap. Quote from glibc (C-library) "
"documentation (http://www.gnu.org/software/libc/manual/html_mono/libc.html#Formatted-Output-Functions): "
"\"If copying takes place between objects that overlap as a result of a call "
"to sprintf() or snprintf(), the results are undefined.\"");
}

119
lib/checkstring.h Normal file
View File

@ -0,0 +1,119 @@
/*
* 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/>.
*/
//---------------------------------------------------------------------------
#ifndef checkstringH
#define checkstringH
//---------------------------------------------------------------------------
#include "config.h"
#include "check.h"
/// @addtogroup Checks
/// @{
/** @brief Detect misusage of C-style strings and related standard functions */
class CPPCHECKLIB CheckString : public Check {
public:
/** @brief This constructor is used when registering the CheckClass */
CheckString() : Check(myName()) {
}
/** @brief This constructor is used when running checks. */
CheckString(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
: Check(myName(), tokenizer, settings, errorLogger) {
}
/** @brief Run checks against the normal token list */
void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
CheckString checkString(tokenizer, settings, errorLogger);
// Checks
checkString.strPlusChar();
checkString.checkSuspiciousStringCompare();
}
/** @brief Run checks against the simplified token list */
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
CheckString checkString(tokenizer, settings, errorLogger);
// Checks
checkString.checkIncorrectStringCompare();
checkString.checkAlwaysTrueOrFalseStringCompare();
checkString.sprintfOverlappingData();
}
/** @brief str plus char (unusual pointer arithmetic) */
void strPlusChar();
/** @brief %Check for using bad usage of strncmp and substr */
void checkIncorrectStringCompare();
/** @brief %Check for comparison of a string literal with a char* variable */
void checkSuspiciousStringCompare();
/** @brief %Check for suspicious code that compares string literals for equality */
void checkAlwaysTrueOrFalseStringCompare();
/** @brief %Check for overlapping source and destination passed to sprintf() */
void sprintfOverlappingData();
private:
void sprintfOverlappingDataError(const Token *tok, const std::string &varname);
void strPlusCharError(const Token *tok);
void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string);
void incorrectStringBooleanError(const Token *tok, const std::string& string);
void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
void suspiciousStringCompareError(const Token* tok, const std::string& var);
void suspiciousStringCompareError_char(const Token* tok, const std::string& var);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
CheckString c(0, settings, errorLogger);
c.sprintfOverlappingDataError(0, "varname");
c.strPlusCharError(0);
c.incorrectStringCompareError(0, "substr", "\"Hello World\"");
c.suspiciousStringCompareError(0, "foo");
c.suspiciousStringCompareError_char(0, "foo");
c.incorrectStringBooleanError(0, "\"Hello World\"");
c.alwaysTrueFalseStringCompareError(0, "str1", "str2");
c.alwaysTrueStringVariableCompareError(0, "varname1", "varname2");
}
static std::string myName() {
return "String";
}
std::string classInfo() const {
return "Detect misusage of C-style strings:\n"
"* overlapping buffers passed to sprintf as source and destination\n"
"* incorrect length arguments for 'substr' and 'strncmp'\n"
"* suspicious condition (runtime comparison of string literals)\n"
"* suspicious condition (string literals as boolean)\n"
"* suspicious comparison of a string literal with a char* variable\n"
"* suspicious comparison of '\\0' with a char* variable\n";
}
};
/// @}
//---------------------------------------------------------------------------
#endif // checkstringH

2
lib/cppcheck.vcxproj
View File

@ -45,6 +45,7 @@
<ClCompile Include="checkboost.cpp" /> <ClCompile Include="checkboost.cpp" />
<ClCompile Include="checkbufferoverrun.cpp" /> <ClCompile Include="checkbufferoverrun.cpp" />
<ClCompile Include="checkclass.cpp" /> <ClCompile Include="checkclass.cpp" />
<ClCompile Include="checkstring.cpp" />
<ClCompile Include="checkexceptionsafety.cpp" /> <ClCompile Include="checkexceptionsafety.cpp" />
<ClCompile Include="checkinternal.cpp" /> <ClCompile Include="checkinternal.cpp" />
<ClCompile Include="checkio.cpp" /> <ClCompile Include="checkio.cpp" />
@ -89,6 +90,7 @@
<ClInclude Include="checkboost.h" /> <ClInclude Include="checkboost.h" />
<ClInclude Include="checkbufferoverrun.h" /> <ClInclude Include="checkbufferoverrun.h" />
<ClInclude Include="checkclass.h" /> <ClInclude Include="checkclass.h" />
<ClInclude Include="checkstring.h" />
<ClInclude Include="checkexceptionsafety.h" /> <ClInclude Include="checkexceptionsafety.h" />
<ClInclude Include="checkinternal.h" /> <ClInclude Include="checkinternal.h" />
<ClInclude Include="checkio.h" /> <ClInclude Include="checkio.h" />

6
lib/cppcheck.vcxproj.filters
View File

@ -137,6 +137,9 @@
<ClCompile Include="checkvaarg.cpp"> <ClCompile Include="checkvaarg.cpp">
<Filter>Source Files</Filter> <Filter>Source Files</Filter>
</ClCompile> </ClCompile>
<ClCompile Include="checkstring.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup> </ItemGroup>
<ItemGroup> <ItemGroup>
<ClInclude Include="checkbufferoverrun.h"> <ClInclude Include="checkbufferoverrun.h">
@ -271,6 +274,9 @@
<ClInclude Include="checkvaarg.h"> <ClInclude Include="checkvaarg.h">
<Filter>Header Files</Filter> <Filter>Header Files</Filter>
</ClInclude> </ClInclude>
<ClInclude Include="checkstring.h">
<Filter>Header Files</Filter>
</ClInclude>
</ItemGroup> </ItemGroup>
<ItemGroup> <ItemGroup>
<ResourceCompile Include="version.rc" /> <ResourceCompile Include="version.rc" />

477
test/testother.cpp
View File

@ -21,6 +21,7 @@
#include "symboldatabase.h" #include "symboldatabase.h"
#include "checkother.h" #include "checkother.h"
#include "testsuite.h" #include "testsuite.h"
#include "testutils.h"
#include <sstream> #include <sstream>
#include <tinyxml2.h> #include <tinyxml2.h>
@ -53,16 +54,6 @@ private:
TEST_CASE(invalidFunctionUsage1); TEST_CASE(invalidFunctionUsage1);
TEST_CASE(sprintf1); // Dangerous usage of sprintf
TEST_CASE(sprintf2);
TEST_CASE(sprintf3);
TEST_CASE(sprintf4); // struct member
TEST_CASE(strPlusChar1); // "/usr" + '/'
TEST_CASE(strPlusChar2); // "/usr" + ch
TEST_CASE(strPlusChar3); // ok: path + "/sub" + '/'
TEST_CASE(strPlusChar4); // ast
TEST_CASE(varScope1); TEST_CASE(varScope1);
TEST_CASE(varScope2); TEST_CASE(varScope2);
TEST_CASE(varScope3); TEST_CASE(varScope3);
@ -153,8 +144,6 @@ private:
TEST_CASE(clarifyCondition5); // #3609 CWinTraits<WS_CHILD|WS_VISIBLE>.. TEST_CASE(clarifyCondition5); // #3609 CWinTraits<WS_CHILD|WS_VISIBLE>..
TEST_CASE(clarifyCondition6); // #3818 TEST_CASE(clarifyCondition6); // #3818
TEST_CASE(incorrectStringCompare);
TEST_CASE(duplicateBranch); 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(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 TEST_CASE(duplicateBranch2); // empty macro
@ -165,9 +154,6 @@ private:
TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values) TEST_CASE(duplicateExpression5); // ticket #3749 (macros with same values)
TEST_CASE(duplicateExpression6); // ticket #4639 TEST_CASE(duplicateExpression6); // ticket #4639
TEST_CASE(alwaysTrueFalseStringCompare);
TEST_CASE(suspiciousStringCompare);
TEST_CASE(suspiciousStringCompare_char);
TEST_CASE(checkSignOfUnsignedVariable); TEST_CASE(checkSignOfUnsignedVariable);
TEST_CASE(checkSignOfPointer); TEST_CASE(checkSignOfPointer);
@ -246,23 +232,6 @@ private:
} }
} }
class SimpleSuppressor: public ErrorLogger {
public:
SimpleSuppressor(Settings &settings, ErrorLogger *next)
: _settings(settings), _next(next) {
}
virtual void reportOut(const std::string &outmsg) {
_next->reportOut(outmsg);
}
virtual void reportErr(const ErrorLogger::ErrorMessage &msg) {
if (!msg._callStack.empty() && !_settings.nomsg.isSuppressed(msg._id, msg._callStack.begin()->getfile(), msg._callStack.begin()->line))
_next->reportErr(msg);
}
private:
Settings &_settings;
ErrorLogger *_next;
};
void check_preprocess_suppress(const char precode[], const char *filename = nullptr) { void check_preprocess_suppress(const char precode[], const char *filename = nullptr) {
// Clear the error buffer.. // Clear the error buffer..
errout.str(""); errout.str("");
@ -293,7 +262,6 @@ private:
// Check.. // Check..
CheckOther checkOther(&tokenizer, &settings, &logger); CheckOther checkOther(&tokenizer, &settings, &logger);
checkOther.checkSwitchCaseFallThrough(); checkOther.checkSwitchCaseFallThrough();
checkOther.checkAlwaysTrueOrFalseStringCompare();
logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(filename)); logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(filename));
} }
@ -804,113 +772,6 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void sprintf1() {
invalidFunctionUsage("void foo()\n"
"{\n"
" char buf[100];\n"
" sprintf(buf,\"%s\",buf);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Undefined behavior: Variable 'buf' is used as parameter and destination in s[n]printf().\n", errout.str());
}
void sprintf2() {
invalidFunctionUsage("void foo()\n"
"{\n"
" char buf[100];\n"
" sprintf(buf,\"%i\",sizeof(buf));\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf3() {
invalidFunctionUsage("void foo()\n"
"{\n"
" char buf[100];\n"
" sprintf(buf,\"%i\",sizeof(buf));\n"
" if (buf[0]);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf4() {
invalidFunctionUsage("struct A\n"
"{\n"
" char filename[128];\n"
"};\n"
"\n"
"void foo()\n"
"{\n"
" const char* filename = \"hello\";\n"
" struct A a;\n"
" snprintf(a.filename, 128, \"%s\", filename);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void strPlusChar(const char code[]) {
// Clear the error buffer..
errout.str("");
Settings settings;
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
// Check for redundant code..
CheckOther checkOther(&tokenizer, &settings, this);
checkOther.strPlusChar();
}
void strPlusChar1() {
// Strange looking pointer arithmetic..
strPlusChar("void foo()\n"
"{\n"
" const char *p = \"/usr\" + '/';\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Unusual pointer arithmetic. A value of type 'char' is added to a string literal.\n", errout.str());
}
void strPlusChar2() {
// Strange looking pointer arithmetic..
strPlusChar("void foo()\n"
"{\n"
" char ch = 1;\n"
" const char *p = ch + \"/usr\";\n"
"}");
ASSERT_EQUALS("", errout.str());
// Strange looking pointer arithmetic..
strPlusChar("void foo()\n"
"{\n"
" int i = 1;\n"
" const char* psz = \"Bla\";\n"
" const std::string str = i + psz;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void strPlusChar3() {
// Strange looking pointer arithmetic..
strPlusChar("void foo()\n"
"{\n"
" std::string temp = \"/tmp\";\n"
" std::string path = temp + '/' + \"sub\" + '/';\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void strPlusChar4() {
// don't crash
strPlusChar("int test() { int +; }");
}
void varScope(const char code[]) { void varScope(const char code[]) {
// Clear the error buffer.. // Clear the error buffer..
errout.str(""); errout.str("");
@ -4625,83 +4486,6 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void incorrectStringCompare() {
check("int f() {\n"
" return test.substr( 0 , 4 ) == \"Hello\" ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"Hello\" doesn't match length argument for substr().\n", errout.str());
check("int f() {\n"
" return test.substr( 0 , 5 ) == \"Hello\" ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" return \"Hello\" == test.substr( 0 , 4 ) ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"Hello\" doesn't match length argument for substr().\n", errout.str());
check("int f() {\n"
" return \"Hello\" == foo.bar<int>().z[1].substr(i+j*4, 4) ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"Hello\" doesn't match length argument for substr().\n", errout.str());
check("int f() {\n"
" return \"Hello\" == test.substr( 0 , 5 ) ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" if (\"Hello\") { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" if (\"Hello\" && test) { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" if (test && \"Hello\") { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" while (\"Hello\") { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" assert (test || \"Hello\");\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" assert (test && \"Hello\");\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" assert (\"Hello\" || test);\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" assert (\"Hello\" && test);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" BOOST_ASSERT (\"Hello\" && test);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" return f2(\"Hello\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void duplicateBranch() { void duplicateBranch() {
check("void f(int a, int &b) {\n" check("void f(int a, int &b) {\n"
" if (a)\n" " if (a)\n"
@ -5142,265 +4926,6 @@ private:
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
} }
void alwaysTrueFalseStringCompare() {
check_preprocess_suppress(
"#define MACRO \"00FF00\"\n"
"int main()\n"
"{\n"
" if (strcmp(MACRO,\"00FF00\") == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main()\n"
"{\n"
" if (stricmp(\"hotdog\",\"HOTdog\") == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"#define MACRO \"Hotdog\"\n"
"int main()\n"
"{\n"
" if (QString::compare(\"Hamburger\", MACRO) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main()\n"
"{\n"
" if (QString::compare(argv[2], \"hotdog\") == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("", errout.str());
check_preprocess_suppress(
"int main()\n"
"{\n"
" if (strncmp(\"hotdog\",\"hotdog\", 6) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check(
"int foo(const char *buf)\n"
"{\n"
" if (strcmp(buf, buf) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of identical string variables.\n", errout.str());
check(
"int foo(const std::string& buf)\n"
"{\n"
" if (stricmp(buf.c_str(), buf.c_str()) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of identical string variables.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
" if (\"str\" == \"str\") {\n"
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
" if (\"str\" != \"str\") {\n"
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
" if (a+\"str\" != \"str\"+b) {\n"
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void suspiciousStringCompare() {
check("bool foo(char* c) {\n"
" return c == \"x\";\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(const char* c) {\n"
" return \"x\" == c;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(char* c) {\n"
" return foo+\"x\" == c;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return \"x\" == c+foo;\n"
"}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return \"x\" == c+foo;\n"
"}", "test.c");
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(Foo c) {\n"
" return \"x\" == c.foo;\n"
"}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("bool foo(Foo c) {\n"
" return \"x\" == c.foo;\n"
"}", "test.c");
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c.foo'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(const std::string& c) {\n"
" return \"x\" == c;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(const Foo* c) {\n"
" return \"x\" == c->bar();\n" // #4314
"}");
ASSERT_EQUALS("", errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString *str=Getter();\n"
"return *str==\"bug\"; }\n", "test.c");
ASSERT_EQUALS("[test.c:3]: (warning) String literal compared with variable '*str'. Did you intend to use strcmp() instead?\n", errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString *str=Getter();\n"
"return *str==\"bug\"; }");
ASSERT_EQUALS("", errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString **str=OtherGetter();\n"
"return *str==\"bug\"; }");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n",
"",
errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString str=OtherGetter2();\n"
"return &str==\"bug\"; }");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n",
"",
errout.str());
// Ticket #5734
check("int foo(char c) {\n"
"return c == '42';}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("int foo(char c) {\n"
"return c == '42';}", "test.c");
ASSERT_EQUALS("", errout.str());
check("int foo(char c) {\n"
"return c == \"42\"[0];}", "test.cpp", false, true, false, false);
ASSERT_EQUALS("", errout.str());
check("int foo(char c) {\n"
"return c == \"42\"[0];}", "test.c", false, true, false, false);
ASSERT_EQUALS("", errout.str());
// 5639 String literal compared with char buffer in a struct
check("struct Example {\n"
" char buffer[200];\n"
"};\n"
"void foo() {\n"
" struct Example example;\n"
" if (example.buffer == \"test\") ;\n"
"}\n", "test.cpp", false, true, false, false);
ASSERT_EQUALS("[test.cpp:6]: (warning) String literal compared with variable 'example.buffer'. Did you intend to use strcmp() instead?\n", errout.str());
check("struct Example {\n"
" char buffer[200];\n"
"};\n"
"void foo() {\n"
" struct Example example;\n"
" if (example.buffer == \"test\") ;\n"
"}\n", "test.c", false, true, false, false);
ASSERT_EQUALS("[test.c:6]: (warning) String literal compared with variable 'example.buffer'. Did you intend to use strcmp() instead?\n", errout.str());
}
void suspiciousStringCompare_char() {
check("bool foo(char* c) {\n"
" return c == '\\0';\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str());
check("bool foo(char* c) {\n"
" return '\\0' != c;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str());
check("bool foo(char c) {\n"
" return c == '\\0';\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(char c) {\n"
" return c == 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return *c == 0;\n"
"}", "test.c");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return *c == 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(Foo* c) {\n"
" return 0 == c->x;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void foo(char* c) {\n"
" if(c == '\\0') bar();\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", "", errout.str());
check("void f() {\n"
" struct { struct { char *str; } x; } a;\n"
" return a.x.str == '\\0';"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Char literal compared with pointer 'a.x.str'. Did you intend to dereference it?\n", errout.str());
check("void f() {\n"
" struct { struct { char *str; } x; } a;\n"
" return *a.x.str == '\\0';"
"}");
ASSERT_EQUALS("", errout.str());
}
void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) { void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) {
// Clear the error buffer.. // Clear the error buffer..
errout.str(""); errout.str("");

1
test/testrunner.vcxproj
View File

@ -67,6 +67,7 @@
<ClCompile Include="testsimplifytokens.cpp" /> <ClCompile Include="testsimplifytokens.cpp" />
<ClCompile Include="testsizeof.cpp" /> <ClCompile Include="testsizeof.cpp" />
<ClCompile Include="teststl.cpp" /> <ClCompile Include="teststl.cpp" />
<ClCompile Include="teststring.cpp" />
<ClCompile Include="testsuite.cpp"> <ClCompile Include="testsuite.cpp">
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">Create</PrecompiledHeader> <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">Create</PrecompiledHeader>
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">Create</PrecompiledHeader> <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">Create</PrecompiledHeader>

3
test/testrunner.vcxproj.filters
View File

@ -187,6 +187,9 @@
<ClCompile Include="testvaarg.cpp"> <ClCompile Include="testvaarg.cpp">
<Filter>Source Files</Filter> <Filter>Source Files</Filter>
</ClCompile> </ClCompile>
<ClCompile Include="teststring.cpp">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup> </ItemGroup>
<ItemGroup> <ItemGroup>
<ClInclude Include="options.h"> <ClInclude Include="options.h">

528
test/teststring.cpp Normal file
View File

@ -0,0 +1,528 @@
/*
* 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 "checkstring.h"
#include "testsuite.h"
#include "preprocessor.h"
#include "testutils.h"
#include <sstream>
extern std::ostringstream errout;
class TestString : public TestFixture {
public:
TestString() : TestFixture("TestString") {
}
private:
void run() {
TEST_CASE(alwaysTrueFalseStringCompare);
TEST_CASE(suspiciousStringCompare);
TEST_CASE(suspiciousStringCompare_char);
TEST_CASE(strPlusChar1); // "/usr" + '/'
TEST_CASE(strPlusChar2); // "/usr" + ch
TEST_CASE(strPlusChar3); // ok: path + "/sub" + '/'
TEST_CASE(strPlusChar4); // ast
TEST_CASE(sprintf1); // Dangerous usage of sprintf
TEST_CASE(sprintf2);
TEST_CASE(sprintf3);
TEST_CASE(sprintf4); // struct member
TEST_CASE(incorrectStringCompare);
}
void check(const char code[], const char filename[] = "test.cpp") {
// Clear the error buffer..
errout.str("");
Settings settings;
settings.addEnabled("warning");
settings.addEnabled("style");
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code);
tokenizer.tokenize(istr, filename);
// Check char variable usage..
CheckString checkString(&tokenizer, &settings, this);
checkString.runChecks(&tokenizer, &settings, this);
tokenizer.simplifyTokenList2();
checkString.runSimplifiedChecks(&tokenizer, &settings, this);
}
void check_preprocess_suppress(const char precode[]) {
// Clear the error buffer..
errout.str("");
Settings settings;
settings.addEnabled("warning");
// Preprocess file..
SimpleSuppressor logger(settings, this);
Preprocessor preprocessor(&settings, &logger);
std::list<std::string> configurations;
std::string filedata;
std::istringstream fin(precode);
preprocessor.preprocess(fin, filedata, configurations, "test.cpp", settings._includePaths);
const std::string code = preprocessor.getcode(filedata, "", "test.cpp");
// Tokenize..
Tokenizer tokenizer(&settings, &logger);
std::istringstream istr(code);
tokenizer.tokenize(istr, "test.cpp");
// Check..
CheckString checkString(&tokenizer, &settings, &logger);
checkString.checkAlwaysTrueOrFalseStringCompare();
logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions("test.cpp"));
}
void alwaysTrueFalseStringCompare() {
check_preprocess_suppress(
"#define MACRO \"00FF00\"\n"
"int main()\n"
"{\n"
" if (strcmp(MACRO,\"00FF00\") == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main()\n"
"{\n"
" if (stricmp(\"hotdog\",\"HOTdog\") == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"#define MACRO \"Hotdog\"\n"
"int main()\n"
"{\n"
" if (QString::compare(\"Hamburger\", MACRO) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:4]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main()\n"
"{\n"
" if (QString::compare(argv[2], \"hotdog\") == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("", errout.str());
check_preprocess_suppress(
"int main()\n"
"{\n"
" if (strncmp(\"hotdog\",\"hotdog\", 6) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check(
"int foo(const char *buf)\n"
"{\n"
" if (strcmp(buf, buf) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of identical string variables.\n", errout.str());
check(
"int foo(const std::string& buf)\n"
"{\n"
" if (stricmp(buf.c_str(), buf.c_str()) == 0)"
" {"
" std::cout << \"Equal\n\""
" }"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Comparison of identical string variables.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
" if (\"str\" == \"str\") {\n"
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
" if (\"str\" != \"str\") {\n"
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Unnecessary comparison of static strings.\n", errout.str());
check_preprocess_suppress(
"int main() {\n"
" if (a+\"str\" != \"str\"+b) {\n"
" std::cout << \"Equal\n\"\n"
" }\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void suspiciousStringCompare() {
check("bool foo(char* c) {\n"
" return c == \"x\";\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(const char* c) {\n"
" return \"x\" == c;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(char* c) {\n"
" return foo+\"x\" == c;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return \"x\" == c+foo;\n"
"}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return \"x\" == c+foo;\n"
"}", "test.c");
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(Foo c) {\n"
" return \"x\" == c.foo;\n"
"}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("bool foo(Foo c) {\n"
" return \"x\" == c.foo;\n"
"}", "test.c");
ASSERT_EQUALS("[test.c:2]: (warning) String literal compared with variable 'c.foo'. Did you intend to use strcmp() instead?\n", errout.str());
check("bool foo(const std::string& c) {\n"
" return \"x\" == c;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(const Foo* c) {\n"
" return \"x\" == c->bar();\n" // #4314
"}");
ASSERT_EQUALS("", errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString *str=Getter();\n"
"return *str==\"bug\"; }\n", "test.c");
ASSERT_EQUALS("[test.c:3]: (warning) String literal compared with variable '*str'. Did you intend to use strcmp() instead?\n", errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString *str=Getter();\n"
"return *str==\"bug\"; }");
ASSERT_EQUALS("", errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString **str=OtherGetter();\n"
"return *str==\"bug\"; }");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n",
"",
errout.str());
// Ticket #4257
check("bool foo() {\n"
"MyString str=OtherGetter2();\n"
"return &str==\"bug\"; }");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n",
"",
errout.str());
// Ticket #5734
check("int foo(char c) {\n"
"return c == '42';}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("int foo(char c) {\n"
"return c == '42';}", "test.c");
ASSERT_EQUALS("", errout.str());
check("int foo(char c) {\n"
"return c == \"42\"[0];}", "test.cpp");
ASSERT_EQUALS("", errout.str());
check("int foo(char c) {\n"
"return c == \"42\"[0];}", "test.c");
ASSERT_EQUALS("", errout.str());
// 5639 String literal compared with char buffer in a struct
check("struct Example {\n"
" char buffer[200];\n"
"};\n"
"void foo() {\n"
" struct Example example;\n"
" if (example.buffer == \"test\") ;\n"
"}\n", "test.cpp");
ASSERT_EQUALS("[test.cpp:6]: (warning) String literal compared with variable 'example.buffer'. Did you intend to use strcmp() instead?\n", errout.str());
check("struct Example {\n"
" char buffer[200];\n"
"};\n"
"void foo() {\n"
" struct Example example;\n"
" if (example.buffer == \"test\") ;\n"
"}\n", "test.c");
ASSERT_EQUALS("[test.c:6]: (warning) String literal compared with variable 'example.buffer'. Did you intend to use strcmp() instead?\n", errout.str());
}
void suspiciousStringCompare_char() {
check("bool foo(char* c) {\n"
" return c == '\\0';\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str());
check("bool foo(char* c) {\n"
" return '\\0' != c;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str());
check("bool foo(char c) {\n"
" return c == '\\0';\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(char c) {\n"
" return c == 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return *c == 0;\n"
"}", "test.c");
ASSERT_EQUALS("", errout.str());
check("bool foo(char* c) {\n"
" return *c == 0;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("bool foo(Foo* c) {\n"
" return 0 == c->x;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("void foo(char* c) {\n"
" if(c == '\\0') bar();\n"
"}");
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", "", errout.str());
check("void f() {\n"
" struct { struct { char *str; } x; } a;\n"
" return a.x.str == '\\0';"
"}");
ASSERT_EQUALS("[test.cpp:3]: (warning) Char literal compared with pointer 'a.x.str'. Did you intend to dereference it?\n", errout.str());
check("void f() {\n"
" struct { struct { char *str; } x; } a;\n"
" return *a.x.str == '\\0';"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf1() {
check("void foo()\n"
"{\n"
" char buf[100];\n"
" sprintf(buf,\"%s\",buf);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Undefined behavior: Variable 'buf' is used as parameter and destination in s[n]printf().\n", errout.str());
}
void sprintf2() {
check("void foo()\n"
"{\n"
" char buf[100];\n"
" sprintf(buf,\"%i\",sizeof(buf));\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf3() {
check("void foo()\n"
"{\n"
" char buf[100];\n"
" sprintf(buf,\"%i\",sizeof(buf));\n"
" if (buf[0]);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void sprintf4() {
check("struct A\n"
"{\n"
" char filename[128];\n"
"};\n"
"\n"
"void foo()\n"
"{\n"
" const char* filename = \"hello\";\n"
" struct A a;\n"
" snprintf(a.filename, 128, \"%s\", filename);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void strPlusChar1() {
// Strange looking pointer arithmetic..
check("void foo()\n"
"{\n"
" const char *p = \"/usr\" + '/';\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) Unusual pointer arithmetic. A value of type 'char' is added to a string literal.\n", errout.str());
}
void strPlusChar2() {
// Strange looking pointer arithmetic..
check("void foo()\n"
"{\n"
" char ch = 1;\n"
" const char *p = ch + \"/usr\";\n"
"}");
ASSERT_EQUALS("", errout.str());
// Strange looking pointer arithmetic..
check("void foo()\n"
"{\n"
" int i = 1;\n"
" const char* psz = \"Bla\";\n"
" const std::string str = i + psz;\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void strPlusChar3() {
// Strange looking pointer arithmetic..
check("void foo()\n"
"{\n"
" std::string temp = \"/tmp\";\n"
" std::string path = temp + '/' + \"sub\" + '/';\n"
"}");
ASSERT_EQUALS("", errout.str());
}
void strPlusChar4() {
// don't crash
check("int test() { int +; }");
}
void incorrectStringCompare() {
check("int f() {\n"
" return test.substr( 0 , 4 ) == \"Hello\" ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"Hello\" doesn't match length argument for substr().\n", errout.str());
check("int f() {\n"
" return test.substr( 0 , 5 ) == \"Hello\" ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" return \"Hello\" == test.substr( 0 , 4 ) ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"Hello\" doesn't match length argument for substr().\n", errout.str());
check("int f() {\n"
" return \"Hello\" == foo.bar<int>().z[1].substr(i+j*4, 4) ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"Hello\" doesn't match length argument for substr().\n", errout.str());
check("int f() {\n"
" return \"Hello\" == test.substr( 0 , 5 ) ? : 0 : 1 ;\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" if (\"Hello\") { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" if (\"Hello\" && test) { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" if (test && \"Hello\") { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" while (\"Hello\") { }\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" assert (test || \"Hello\");\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" assert (test && \"Hello\");\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" assert (\"Hello\" || test);\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (warning) Conversion of string literal \"Hello\" to bool always evaluates to true.\n", errout.str());
check("int f() {\n"
" assert (\"Hello\" && test);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" BOOST_ASSERT (\"Hello\" && test);\n"
"}");
ASSERT_EQUALS("", errout.str());
check("int f() {\n"
" return f2(\"Hello\");\n"
"}");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestString)

18
test/testutils.h
View File

@ -44,4 +44,22 @@ public:
} }
}; };
class SimpleSuppressor : public ErrorLogger {
public:
SimpleSuppressor(Settings &settings, ErrorLogger *next)
: _settings(settings), _next(next) {
}
virtual void reportOut(const std::string &outmsg) {
_next->reportOut(outmsg);
}
virtual void reportErr(const ErrorLogger::ErrorMessage &msg) {
if (!msg._callStack.empty() && !_settings.nomsg.isSuppressed(msg._id, msg._callStack.begin()->getfile(), msg._callStack.begin()->line))
_next->reportErr(msg);
}
private:
Settings &_settings;
ErrorLogger *_next;
};
#endif // TestUtilsH #endif // TestUtilsH