diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 0a6471d3f..0187bab93 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -1009,7 +1009,7 @@ void CheckIO::checkFormatString(const Token * const tok, } else if (argInfo.typeToken->str() != "long" || argInfo.typeToken->isLong()) invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); else if (typesMatch(argInfo.typeToken->originalName(), "size_t") || - argInfo.typeToken->originalName() == "uintmax_t") + argInfo.typeToken->originalName() == "uintmax_t") invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); break; case 'j': diff --git a/lib/checkstring.cpp b/lib/checkstring.cpp index 678a00da2..f6ab41d21 100644 --- a/lib/checkstring.cpp +++ b/lib/checkstring.cpp @@ -346,75 +346,75 @@ void CheckString::overlappingStringComparisons() 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() != "||") - continue; - std::list equals0; - std::list notEquals0; - std::stack tokens; - tokens.push(tok); - while (!tokens.empty()) { - const Token * const t = tokens.top(); - tokens.pop(); - if (!t) - continue; - if (t->str() == "||") { - tokens.push(t->astOperand1()); - tokens.push(t->astOperand2()); - continue; - } - if (t->str() == "==") { - if (Token::simpleMatch(t->astOperand1(), "(") && Token::simpleMatch(t->astOperand2(), "0")) - equals0.push_back(t->astOperand1()); - else if (Token::simpleMatch(t->astOperand2(), "(") && Token::simpleMatch(t->astOperand1(), "0")) - equals0.push_back(t->astOperand2()); - continue; - } - if (t->str() == "!=") { - if (Token::simpleMatch(t->astOperand1(), "(") && Token::simpleMatch(t->astOperand2(), "0")) - notEquals0.push_back(t->astOperand1()); - else if (Token::simpleMatch(t->astOperand2(), "(") && Token::simpleMatch(t->astOperand1(), "0")) - notEquals0.push_back(t->astOperand2()); - continue; - } - if (t->str() == "!" && Token::simpleMatch(t->astOperand1(), "(")) - equals0.push_back(t->astOperand1()); - else if (t->str() == "(") - notEquals0.push_back(t); - } + if (tok->str() != "||") + continue; + std::list equals0; + std::list notEquals0; + std::stack tokens; + tokens.push(tok); + while (!tokens.empty()) { + const Token * const t = tokens.top(); + tokens.pop(); + if (!t) + continue; + if (t->str() == "||") { + tokens.push(t->astOperand1()); + tokens.push(t->astOperand2()); + continue; + } + if (t->str() == "==") { + if (Token::simpleMatch(t->astOperand1(), "(") && Token::simpleMatch(t->astOperand2(), "0")) + equals0.push_back(t->astOperand1()); + else if (Token::simpleMatch(t->astOperand2(), "(") && Token::simpleMatch(t->astOperand1(), "0")) + equals0.push_back(t->astOperand2()); + continue; + } + if (t->str() == "!=") { + if (Token::simpleMatch(t->astOperand1(), "(") && Token::simpleMatch(t->astOperand2(), "0")) + notEquals0.push_back(t->astOperand1()); + else if (Token::simpleMatch(t->astOperand2(), "(") && Token::simpleMatch(t->astOperand1(), "0")) + notEquals0.push_back(t->astOperand2()); + continue; + } + if (t->str() == "!" && Token::simpleMatch(t->astOperand1(), "(")) + equals0.push_back(t->astOperand1()); + else if (t->str() == "(") + notEquals0.push_back(t); + } - bool error = false; - for (std::list::const_iterator eq0 = equals0.begin(); !error && eq0 != equals0.end(); ++eq0) { - for (std::list::const_iterator ne0 = notEquals0.begin(); !error && ne0 != notEquals0.end(); ++ne0) { - const Token *tok1 = *eq0; - const Token *tok2 = *ne0; - if (!Token::simpleMatch(tok1->previous(), "strcmp (")) - continue; - if (!Token::simpleMatch(tok2->previous(), "strcmp (")) - continue; - const std::vector args1 = getArguments(tok1->previous()); - const std::vector args2 = getArguments(tok2->previous()); - if (args1.size() != 2 || args2.size() != 2) - continue; - if (args1[1]->isLiteral() && - args2[1]->isLiteral() && - args1[1]->str() != args2[1]->str() && - isSameExpression(_tokenizer->isCPP(), true, args1[0], args2[0], _settings->library, true)) - overlappingStringComparisonsError(tok1, tok2); - } - } - } - } + bool error = false; + for (std::list::const_iterator eq0 = equals0.begin(); !error && eq0 != equals0.end(); ++eq0) { + for (std::list::const_iterator ne0 = notEquals0.begin(); !error && ne0 != notEquals0.end(); ++ne0) { + const Token *tok1 = *eq0; + const Token *tok2 = *ne0; + if (!Token::simpleMatch(tok1->previous(), "strcmp (")) + continue; + if (!Token::simpleMatch(tok2->previous(), "strcmp (")) + continue; + const std::vector args1 = getArguments(tok1->previous()); + const std::vector args2 = getArguments(tok2->previous()); + if (args1.size() != 2 || args2.size() != 2) + continue; + if (args1[1]->isLiteral() && + args2[1]->isLiteral() && + args1[1]->str() != args2[1]->str() && + isSameExpression(_tokenizer->isCPP(), true, args1[0], args2[0], _settings->library, true)) + overlappingStringComparisonsError(tok1, tok2); + } + } + } + } } void CheckString::overlappingStringComparisonsError(const Token *eq0, const Token *ne0) { - std::string eq0Expr(eq0 ? eq0->expressionString() : std::string("strcmp(x,\"abc\")")); - if (eq0 && eq0->astParent()->str() == "!") - eq0Expr = "!" + eq0Expr; - else - eq0Expr += " == 0"; - const std::string ne0Expr = (ne0 ? ne0->expressionString() : std::string("strcmp(x,\"def\")")) + " != 0"; - reportError(ne0, Severity::warning, "overlappingStringComparisons", "The comparison operator in '" + ne0Expr + "' should maybe be '==' instead, currently the expression '" + eq0Expr + "' is redundant."); + std::string eq0Expr(eq0 ? eq0->expressionString() : std::string("strcmp(x,\"abc\")")); + if (eq0 && eq0->astParent()->str() == "!") + eq0Expr = "!" + eq0Expr; + else + eq0Expr += " == 0"; + const std::string ne0Expr = (ne0 ? ne0->expressionString() : std::string("strcmp(x,\"def\")")) + " != 0"; + reportError(ne0, Severity::warning, "overlappingStringComparisons", "The comparison operator in '" + ne0Expr + "' should maybe be '==' instead, currently the expression '" + eq0Expr + "' is redundant."); } //--------------------------------------------------------------------------- diff --git a/test/testgarbage.cpp b/test/testgarbage.cpp index f1bf42ce1..24fb19c42 100644 --- a/test/testgarbage.cpp +++ b/test/testgarbage.cpp @@ -564,11 +564,11 @@ private: void garbageCode36() { // #6334 ASSERT_THROW(checkCode("{ } < class template < > , { = } ; class... >\n" - "struct Y { }\n" - "class Types { }\n" - "( X < int > \"uses template\" ) ( < ( ) \"uses ;" - "( int int ::primary \"uses template\" ) int double \"uses )" - "::primary , \"uses template\" ;\n"), InternalError); + "struct Y { }\n" + "class Types { }\n" + "( X < int > \"uses template\" ) ( < ( ) \"uses ;" + "( int int ::primary \"uses template\" ) int double \"uses )" + "::primary , \"uses template\" ;\n"), InternalError); } void garbageCode37() { diff --git a/test/teststring.cpp b/test/teststring.cpp index e2339fbb1..05c7baf2f 100644 --- a/test/teststring.cpp +++ b/test/teststring.cpp @@ -51,7 +51,7 @@ private: TEST_CASE(sprintf4); // struct member TEST_CASE(incorrectStringCompare); - + TEST_CASE(overlappingStringComparisons); } @@ -581,13 +581,13 @@ private: ASSERT_EQUALS("", errout.str()); } - void overlappingStringComparisons() { - check("void f(const char *str) {\n" + void overlappingStringComparisons() { + check("void f(const char *str) {\n" " if (strcmp(str, \"abc\") == 0 || strcmp(str, \"def\")) {}\n" "}"); ASSERT_EQUALS("[test.cpp:2]: (warning) The comparison operator in 'strcmp(str,\"def\") != 0' should maybe be '==' instead, currently the expression 'strcmp(str,\"abc\") == 0' is redundant.\n", errout.str()); - check("struct X {\n" + check("struct X {\n" " char *str;\n" "};\n" "\n" @@ -595,7 +595,7 @@ private: " if (strcmp(x->str, \"abc\") == 0 || strcmp(x->str, \"def\")) {}\n" "}"); ASSERT_EQUALS("[test.cpp:6]: (warning) The comparison operator in 'strcmp(x->str,\"def\") != 0' should maybe be '==' instead, currently the expression 'strcmp(x->str,\"abc\") == 0' is redundant.\n", errout.str()); - } + } }; REGISTER_TEST(TestString)