Reverted fix for #4547: It causes fp. See #4573

This commit is contained in:
Daniel Marjamäki 2013-02-12 16:13:08 +01:00
parent 28e38a9e56
commit 1e550f9fdf
6 changed files with 57 additions and 72 deletions

View File

@ -668,57 +668,55 @@ void CheckNullPointer::nullPointerStructByDeRefAndChec()
// count { and } using tok2
const Token* const end2 = tok1->scope()->classEnd;
for (const Token *tok2 = tok1->tokAt(3); tok2 != end2; tok2 = tok2->next()) {
bool unknown = false;
// label / ?:
if (tok2->str() == ":")
break;
// function call..
else {
bool unknown = false;
if (Token::Match(tok2, "[;{}] %var% (") && CanFunctionAssignPointer(tok2->next(), varid1, unknown)) {
if (!_settings->inconclusive || !unknown)
break;
inconclusive = true;
}
// Reassignment of the struct
else if (tok2->varId() == varid1) {
if (tok2->next()->str() == "=") {
// Avoid false positives when there is 'else if'
// TODO: can this be handled better?
if (tok1->strAt(-2) == "if")
skipvar.insert(varid1);
break;
}
if (Token::Match(tok2->tokAt(-2), "[,(] &"))
break;
}
// Loop..
/** @todo don't bail out if the variable is not used in the loop */
else if (tok2->str() == "do")
else if (Token::Match(tok2, "[;{}] %var% (") && CanFunctionAssignPointer(tok2->next(), varid1, unknown)) {
if (!_settings->inconclusive || !unknown)
break;
inconclusive = true;
}
// return/break at base level => stop checking
else if (tok2->scope()->classEnd == end2 && (tok2->str() == "return" || tok2->str() == "break"))
break;
// Function call: If the pointer is not a local variable it
// might be changed by the call.
else if (Token::Match(tok2, "[;{}] %var% (") &&
Token::simpleMatch(tok2->linkAt(2), ") ;") && !isLocal) {
// Reassignment of the struct
else if (tok2->varId() == varid1) {
if (tok2->next()->str() == "=") {
// Avoid false positives when there is 'else if'
// TODO: can this be handled better?
if (tok1->strAt(-2) == "if")
skipvar.insert(varid1);
break;
}
// Check if pointer is null.
// TODO: false negatives for "if (!p || .."
else if (!tok2->isExpandedMacro() && Token::Match(tok2, "if ( !| %varid% )|&&", varid1)) {
// Is this variable a pointer?
if (var->isPointer())
nullPointerError(tok1, varname, tok2, inconclusive);
if (Token::Match(tok2->tokAt(-2), "[,(] &"))
break;
}
}
// Loop..
/** @todo don't bail out if the variable is not used in the loop */
else if (tok2->str() == "do")
break;
// return/break at base level => stop checking
else if (tok2->scope()->classEnd == end2 && (tok2->str() == "return" || tok2->str() == "break"))
break;
// Function call: If the pointer is not a local variable it
// might be changed by the call.
else if (Token::Match(tok2, "[;{}] %var% (") &&
Token::simpleMatch(tok2->linkAt(2), ") ;") && !isLocal) {
break;
}
// Check if pointer is null.
// TODO: false negatives for "if (!p || .."
else if (!tok2->isExpandedMacro() && Token::Match(tok2, "if ( !| %varid% )|&&", varid1)) {
// Is this variable a pointer?
if (var->isPointer())
nullPointerError(tok1, varname, tok2, inconclusive);
break;
}
}
}

View File

@ -2616,14 +2616,14 @@ void CheckOther::checkDuplicateIf()
// save the expression and its location
expressionMap.insert(std::make_pair(expression, tok));
// find the next else { if (...) statement
// find the next else if (...) statement
const Token *tok1 = scope->classEnd;
// check all the else { if (...) statements
while (Token::simpleMatch(tok1, "} else { if (") &&
Token::simpleMatch(tok1->linkAt(4), ") {")) {
// check all the else if (...) statements
while (Token::simpleMatch(tok1, "} else if (") &&
Token::simpleMatch(tok1->linkAt(3), ") {")) {
// get the expression from the token stream
expression = tok1->tokAt(5)->stringifyList(tok1->linkAt(4));
expression = tok1->tokAt(4)->stringifyList(tok1->linkAt(3));
// try to look up the expression to check for duplicates
std::map<std::string, const Token *>::iterator it = expressionMap.find(expression);
@ -2631,7 +2631,7 @@ void CheckOther::checkDuplicateIf()
// found a duplicate
if (it != expressionMap.end()) {
// check for expressions that have side effects and ignore them
if (!expressionHasSideEffects(tok1->tokAt(5), tok1->linkAt(4)->previous()))
if (!expressionHasSideEffects(tok1->tokAt(4), tok1->linkAt(3)->previous()))
duplicateIfError(it->second, tok1->next());
}
@ -2639,8 +2639,8 @@ void CheckOther::checkDuplicateIf()
else
expressionMap.insert(std::make_pair(expression, tok1->next()));
// find the next else { if (...) statement
tok1 = tok1->linkAt(4)->next()->link();
// find the next else if (...) statement
tok1 = tok1->linkAt(3)->next()->link();
}
}
}

View File

@ -71,6 +71,8 @@ std::string Settings::addEnabled(const std::string &str)
return addEnabled(str.substr(prevPos));
}
bool handled = false;
static std::set<std::string> id;
if (id.empty()) {
id.insert("warning");
@ -98,7 +100,7 @@ std::string Settings::addEnabled(const std::string &str)
if (str == "information") {
_enabled.insert("missingInclude");
}
} else {
} else if (!handled) {
if (str.empty())
return std::string("cppcheck: --enable parameter is empty");
else

View File

@ -4101,8 +4101,14 @@ Token *Tokenizer::simplifyAddBracesToCommand(Token *tok)
if (!tokEnd)
return NULL;
Token * tokEndNext=tokEnd->next();
if (tokEndNext && tokEndNext->str()=="else")
tokEnd=simplifyAddBracesPair(tokEndNext,false);
if (tokEndNext && tokEndNext->str()=="else") {
Token * tokEndNextNext=tokEndNext->next();
if (tokEndNextNext && tokEndNextNext->str()=="if") {
// do not change "else if ..." to "else { if ... }"
tokEnd=simplifyAddBracesToCommand(tokEndNextNext);
} else
tokEnd=simplifyAddBracesPair(tokEndNext,false);
}
}
return tokEnd;

View File

@ -4937,12 +4937,6 @@ private:
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Duplicate conditions in 'if' and related 'else if'.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a) { b = 1; }\n"
" else { if (a) { b = 2; } }\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (style) Duplicate conditions in 'if' and related 'else if'.\n", errout.str());
check("void f(int a, int &b) {\n"
" if (a == 1) { b = 1; }\n"
" else if (a == 2) { b = 2; }\n"

View File

@ -108,7 +108,6 @@ private:
TEST_CASE(ifAddBraces17); // '} else' should be in the same line
TEST_CASE(ifAddBraces18); // #3424 - if if { } else else
TEST_CASE(ifAddBraces19); // #3928 - if for if else
TEST_CASE(ifAddBraces20);
TEST_CASE(whileAddBraces);
TEST_CASE(doWhileAddBraces);
@ -1219,20 +1218,6 @@ private:
"}", tokenizeAndStringify(code, true));
}
void ifAddBraces20() {
// if else if
const char code[] = "void f()\n"
"{\n"
" if (a);\n"
" else if (b);\n"
"}\n";
ASSERT_EQUALS("void f ( )\n"
"{\n"
"if ( a ) { ; }\n"
"else { if ( b ) { ; } }\n"
"}", tokenizeAndStringify(code, true));
}
void whileAddBraces() {
const char code[] = ";while(a);";