diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 9ff491e31..c641bfa41 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -215,8 +215,37 @@ void CheckOther::redundantCondition2() } } //--------------------------------------------------------------------------- +// "if (strlen(s))" can be rewritten as "if (*s != '\0')" +//--------------------------------------------------------------------------- +void CheckOther::checkEmptyStringTest() +{ + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + // Non-empty string tests + if (Token::Match(tok, "if ( strlen ( %any% ) )")) + { + emptyStringTestError(tok, tok->strAt(4), false); + } + else if (Token::Match(tok, "strlen ( %any% ) !=|> 0")) + { + emptyStringTestError(tok, tok->strAt(2), false); + } + else if (Token::Match(tok, "0 < strlen ( %any% )")) + { + emptyStringTestError(tok, tok->strAt(4), false); + } - + // Empty string tests + else if (Token::Match(tok, "! strlen ( %any% )")) + { + emptyStringTestError(tok, tok->strAt(3), true); + } + else if (Token::Match(tok, "strlen ( %any% ) == 0")) + { + emptyStringTestError(tok, tok->strAt(2), true); + } + } +} //--------------------------------------------------------------------------- // strtol(str, 0, radix) <- radix must be 0 or 2-36 @@ -2760,3 +2789,17 @@ void CheckOther::postIncrementError(const Token *tok, const std::string &var_nam std::string type = (isIncrement ? "Incrementing" : "Decrementing"); reportError(tok, Severity::possibleStyle, "postIncrementDecrement", ("Pre-" + type + " variable '" + var_name + "' is preferred to Post-" + type)); } + +void CheckOther::emptyStringTestError(const Token *tok, const std::string &var_name, const bool isTestForEmpty) +{ + if (isTestForEmpty) + { + reportError(tok, Severity::possibleStyle, + "emptyStringTest", "Empty string test can be simplified to \"*" + var_name + " == '\\0'\""); + } + else + { + reportError(tok, Severity::possibleStyle, + "emptyStringTest", "Non-empty string test can be simplified to \"*" + var_name + " != '\\0'\""); + } +} diff --git a/lib/checkother.h b/lib/checkother.h index e3936bbdb..1b946a7e7 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -73,6 +73,7 @@ public: checkOther.checkConstantFunctionParameter(); checkOther.checkIncompleteStatement(); checkOther.unreachableCode(); + checkOther.checkEmptyStringTest(); if (settings->inconclusive) { checkOther.postIncrement(); @@ -165,6 +166,8 @@ public: // haystack.remove(needle); void redundantCondition2(); + /** @brief %Check for inefficient empty string test*/ + void checkEmptyStringTest(); // Error messages.. void cstyleCastError(const Token *tok); @@ -191,6 +194,7 @@ public: void zerodivError(const Token *tok); void mathfunctionCallError(const Token *tok, const unsigned int numParam = 1); void postIncrementError(const Token *tok, const std::string &var_name, const bool isIncrement); + void emptyStringTestError(const Token *tok, const std::string &var_name, const bool isTestForEmpty); void getErrorMessages() { @@ -221,6 +225,7 @@ public: // optimisations postIncrementError(0, "varname", true); + emptyStringTestError(0, "varname", true); } std::string name() const diff --git a/test/testother.cpp b/test/testother.cpp index b4580d5f2..fb51e5f5a 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -92,6 +92,8 @@ private: TEST_CASE(passedByValue); TEST_CASE(mathfunctionCall1); + + TEST_CASE(emptyStringTest); } void check(const char code[]) @@ -114,6 +116,7 @@ private: checkOther.checkZeroDivision(); checkOther.unreachableCode(); checkOther.checkMathFunctions(); + checkOther.checkEmptyStringTest(); } @@ -2366,6 +2369,42 @@ private: } + + void emptyStringTest() + { + check("void foo()\n" + "{\n" + " if (strlen(str) == 0)\n" + " {\n" + " std::cout << str;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (possible style) Empty string test can be simplified to \"*str == '\\0'\"\n", errout.str()); + + check("if (!strlen(str)) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Empty string test can be simplified to \"*str == '\\0'\"\n", errout.str()); + + check("if (strlen(str) == 0) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Empty string test can be simplified to \"*str == '\\0'\"\n", errout.str()); + + check("if (strlen(str)) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Non-empty string test can be simplified to \"*str != '\\0'\"\n", errout.str()); + + check("if (strlen(str) > 0) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Non-empty string test can be simplified to \"*str != '\\0'\"\n", errout.str()); + + check("if (strlen(str) != 0) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Non-empty string test can be simplified to \"*str != '\\0'\"\n", errout.str()); + + check("if (0 != strlen(str)) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Non-empty string test can be simplified to \"*str != '\\0'\"\n", errout.str()); + + check("if (0 == strlen(str)) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Empty string test can be simplified to \"*str == '\\0'\"\n", errout.str()); + + check("if (0 < strlen(str)) { }"); + ASSERT_EQUALS("[test.cpp:1]: (possible style) Non-empty string test can be simplified to \"*str != '\\0'\"\n", errout.str()); + } }; REGISTER_TEST(TestOther)