From 4e6d105cbd7f63c47786b35c36128da1d2d0b689 Mon Sep 17 00:00:00 2001 From: PKEuS Date: Tue, 9 Apr 2013 09:16:35 -0700 Subject: [PATCH] Added support for complex patterns to CheckOther::checkIncorrectStringCompare() --- lib/checkother.cpp | 32 ++++++++++++++++++++++---------- test/testother.cpp | 29 +++++++++++++++++------------ 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index ffbc2ee9f..d378c1926 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -2729,17 +2729,29 @@ void CheckOther::checkIncorrectStringCompare() (tok->str().find("assert")+6U==tok->str().size() || tok->str().find("ASSERT")+6U==tok->str().size())) tok = tok->next()->link(); - if (Token::Match(tok, ". substr ( %any% , %num% ) ==|!= %str%")) { - MathLib::bigint clen = MathLib::toLongNumber(tok->strAt(5)); - std::size_t slen = Token::getStrLength(tok->tokAt(8)); - if (clen != (int)slen) { - incorrectStringCompareError(tok->next(), "substr", tok->strAt(8)); + 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; } - } else if (Token::Match(tok, "%str% ==|!= %var% . substr ( %any% , %num% )")) { - MathLib::bigint clen = MathLib::toLongNumber(tok->strAt(8)); - std::size_t slen = Token::getStrLength(tok); - if (clen != (int)slen) { - incorrectStringCompareError(tok->next(), "substr", tok->str()); + 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)); diff --git a/test/testother.cpp b/test/testother.cpp index 661a6cc2e..2d384c926 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -4866,62 +4866,67 @@ private: ASSERT_EQUALS("", errout.str()); check("int f() {\n" - " return \"Hello\" == test.substr( 0 , 4 ) ? : 0 : 1 ;\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\" == test.substr( 0 , 5 ) ? : 0 : 1 ;\n" + " return \"Hello\" == foo.bar().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" + " 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\" && 1) { }\n" + " if (\"Hello\" && 1) { }\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 (1 && \"Hello\") { }\n" + " if (1 && \"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" + " 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 (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 (test && \"Hello\");\n" "}"); ASSERT_EQUALS("", errout.str()); check("int f() {\n" - " assert (\"Hello\" || test);\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 (\"Hello\" && test);\n" "}"); ASSERT_EQUALS("", errout.str()); check("int f() {\n" - " BOOST_ASSERT (\"Hello\" && test);\n" + " BOOST_ASSERT (\"Hello\" && test);\n" "}"); ASSERT_EQUALS("", errout.str()); check("int f() {\n" - " return f2(\"Hello\");\n" + " return f2(\"Hello\");\n" "}"); ASSERT_EQUALS("", errout.str()); }