Fixed #1530 (possible new check: strlen return value versus zero.)

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'\"");
+    }
+}

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

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)