Permalink
Browse files

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

  • Loading branch information...
1 parent 28e38a9 commit 1e550f9fdf0f24a4ff9657fa4fd8e2f8bb3494f9 @danmar committed Feb 12, 2013
Showing with 56 additions and 71 deletions.
  1. +37 −39 lib/checknullpointer.cpp
  2. +8 −8 lib/checkother.cpp
  3. +3 −1 lib/settings.cpp
  4. +8 −2 lib/tokenize.cpp
  5. +0 −6 test/testother.cpp
  6. +0 −15 test/testtokenize.cpp
View
@@ -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;
- }
+ else 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;
+ // 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;
}
-
- // Loop..
- /** @todo don't bail out if the variable is not used in the loop */
- else if (tok2->str() == "do")
+ if (Token::Match(tok2->tokAt(-2), "[,(] &"))
break;
+ }
- // return/break at base level => stop checking
- else if (tok2->scope()->classEnd == end2 && (tok2->str() == "return" || tok2->str() == "break"))
- break;
+ // Loop..
+ /** @todo don't bail out if the variable is not used in the loop */
+ else if (tok2->str() == "do")
+ 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;
- }
+ // return/break at base level => stop checking
+ else if (tok2->scope()->classEnd == end2 && (tok2->str() == "return" || tok2->str() == "break"))
+ 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;
- }
+ // 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
@@ -2616,31 +2616,31 @@ 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);
// 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());
}
// not a duplicate expression so save it and its location
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
@@ -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
@@ -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
@@ -4938,12 +4938,6 @@ class TestOther : public TestFixture {
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"
" else if (a == 1) { b = 3; }\n"
View
@@ -108,7 +108,6 @@ class TestTokenizer : public TestFixture {
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 @@ class TestTokenizer : public TestFixture {
"}", 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);";

0 comments on commit 1e550f9

Please sign in to comment.