diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 8d62349dd..03cd77f71 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -820,9 +820,34 @@ static bool valueFlowForward(Token * const startToken, ++number_of_if; tok2 = end; } else { - if (settings->debugwarnings) - bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in conditional code"); - return false; + bool bail = true; + + // loop that conditionally set variable and then break => either loop condition is + // redundant or the variable can be unchanged after the loop. + bool loopCondition = false; + if (Token::simpleMatch(tok2, "while (") && Token::Match(tok2->next()->astOperand2(), "%op%")) + loopCondition = true; + else if (Token::simpleMatch(tok2, "for (") && + Token::simpleMatch(tok2->next()->astOperand2(), ";") && + Token::simpleMatch(tok2->next()->astOperand2()->astOperand2(), ";") && + Token::Match(tok2->next()->astOperand2()->astOperand2()->astOperand1(), "%op%")) + loopCondition = true; + if (loopCondition) { + const Token *tok3 = Token::findmatch(start, "%varid%", end, varid); + if (Token::Match(tok3, "%varid% =", varid) && + tok3->scope()->classEnd && + Token::Match(tok3->scope()->classEnd->tokAt(-3), "[;}] break ;") && + !Token::findmatch(tok3->next(), "%varid%", end, varid)) { + bail = false; + tok2 = end; + } + } + + if (bail) { + if (settings->debugwarnings) + bailout(tokenlist, errorLogger, tok2, "variable " + var->name() + " is assigned in conditional code"); + return false; + } } } } diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index 412a17b77..d9f1001fd 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -1099,7 +1099,7 @@ private: "\n" " *p = 0;\n" "}"); - TODO_ASSERT_EQUALS("[test.cpp:11]: (error) Possible null pointer dereference: p\n", "", errout.str()); + ASSERT_EQUALS("[test.cpp:11]: (error) Possible null pointer dereference: p\n", errout.str()); } void nullpointer7() { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 3ae3f8c2a..6b02f79a2 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -871,6 +871,43 @@ private: " }\n" "}"; ASSERT_EQUALS(false, testValueOfX(code, 8U, 34)); + + // while/for + code = "void f(const int *buf) {\n" + " int x = 0;\n" + " for (int i = 0; i < 10; i++) {\n" + " if (buf[i] == 123) {\n" + " x = i;\n" + " break;\n" + " }\n" + " }\n" + " a = x;\n" // <- x can be 0 + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 9U, 0)); // x can be 0 at line 9 + + code = "void f(const int *buf) {\n" + " int x = 0;\n" + " for (int i = 0; i < 10; i++) {\n" + " if (buf[i] == 123) {\n" + " x = i;\n" + " ;\n" // <- no break + " }\n" + " }\n" + " a = x;\n" // <- x cant be 0 + "}\n"; + ASSERT_EQUALS(false, testValueOfX(code, 9U, 0)); // x cant be 0 at line 9 + + code = "void f(const int *buf) {\n" + " int x = 0;\n" + " while (++i < 10) {\n" + " if (buf[i] == 123) {\n" + " x = i;\n" + " break;\n" + " }\n" + " }\n" + " a = x;\n" // <- x can be 0 + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 9U, 0)); // x can be 0 at line 9 } void valueFlowAfterCondition() {