New Check: Compare pointer with '\0' (#4070)

lib/checkother.cpp

@@ -2900,16 +2900,18 @@ void CheckOther::checkSuspiciousStringCompare()
             if (Token::simpleMatch(varTok->tokAt(1), "[") || Token::simpleMatch(litTok->tokAt(1), "["))
                 continue;
 
-            if ((varTok->type() == Token::eString || varTok->type() == Token::eVariable) && (litTok->type() == Token::eString || litTok->type() == Token::eVariable) && litTok->type() != varTok->type()) {
+            if (varTok->type() == Token::eString || varTok->type() == Token::eNumber)
-                if (varTok->type() == Token::eString)
+                std::swap(varTok, litTok);
-                    std::swap(varTok, litTok);
+            const Variable *var = varTok->variable();
+            if (!var)
+                continue;
 
-                const Variable *var = varTok->variable();
+            if (_tokenizer->isC() ||
-                if (var) {
+                (var->isPointer() && varTok->strAt(-1) != "*" && !Token::Match(varTok->next(), "[.([]"))) {
-                    if (_tokenizer->isC() ||
+                if (litTok->type() == Token::eString)
-                        (var->isPointer() && varTok->strAt(-1) != "*" && !Token::Match(varTok->next(), "[.([]")))
+                    suspiciousStringCompareError(tok, var->name());
-                        suspiciousStringCompareError(tok, var->name());
+                else if (litTok->type() == Token::eNumber && litTok->originalName() == "'\\0'")
-                }
+                    suspiciousStringCompareError_char(tok, var->name());
             }
         }
     }
@@ -2921,6 +2923,12 @@ void CheckOther::suspiciousStringCompareError(const Token* tok, const std::strin
                 "String literal compared with variable '" + var + "'. Did you intend to use strcmp() instead?");
 }
 
+void CheckOther::suspiciousStringCompareError_char(const Token* tok, const std::string& var)
+{
+    reportError(tok, Severity::warning, "charLiteralWithCharPtrCompare",
+                "Char literal compared with pointer '" + var + "'. Did you intend to dereference it?");
+}
+
 //-----------------------------------------------------------------------------
 // Check is a comparison of two variables leads to condition, which is
 // always true or false.

lib/checkother.h

@@ -316,6 +316,7 @@ private:
     void alwaysTrueFalseStringCompareError(const Token *tok, const std::string& str1, const std::string& str2);
     void alwaysTrueStringVariableCompareError(const Token *tok, const std::string& str1, const std::string& str2);
     void suspiciousStringCompareError(const Token* tok, const std::string& var);
+    void suspiciousStringCompareError_char(const Token* tok, const std::string& var);
     void duplicateBreakError(const Token *tok, bool inconclusive);
     void unreachableCodeError(const Token* tok, bool inconclusive);
     void unsignedLessThanZeroError(const Token *tok, const std::string &varname, bool inconclusive);
@@ -380,6 +381,7 @@ private:
         c.clarifyStatementError(0);
         c.incorrectStringCompareError(0, "substr", "\"Hello World\"");
         c.suspiciousStringCompareError(0, "foo");
+        c.suspiciousStringCompareError_char(0, "foo");
         c.incorrectStringBooleanError(0, "\"Hello World\"");
         c.duplicateBranchError(0, 0);
         c.duplicateExpressionError(0, 0, "&&");
@@ -456,6 +458,7 @@ private:
                "* suspicious condition (runtime comparison of string literals)\n"
                "* suspicious condition (string literals as boolean)\n"
                "* suspicious comparison of a string literal with a char* variable\n"
+               "* suspicious comparison of '\\0' with a char* variable\n"
                "* duplicate break statement\n"
                "* unreachable code\n"
                "* testing if unsigned variable is negative\n"

lib/tokenize.cpp

@@ -1833,11 +1833,12 @@ void Tokenizer::simplifyNull()
     for (Token *tok = list.front(); tok; tok = tok->next()) {
         if (tok->str() == "NULL" && !Token::Match(tok->previous(), "[(,] NULL [,)]"))
             tok->str("0");
-        else if (tok->str() == "__null" || tok->str() == "'\\0'" || tok->str() == "'\\x0'")
+        else if (tok->str() == "__null" || tok->str() == "'\\0'" || tok->str() == "'\\x0'") {
+            tok->originalName(tok->str());
             tok->str("0");
-        else if (tok->isNumber() &&
+        } else if (tok->isNumber() &&
-                 MathLib::isInt(tok->str()) &&
+                   MathLib::isInt(tok->str()) &&
-                 MathLib::toLongNumber(tok->str()) == 0)
+                   MathLib::toLongNumber(tok->str()) == 0)
             tok->str("0");
     }
 
@@ -4394,7 +4395,6 @@ void Tokenizer::simplifyCompoundAssignment()
 
             // Remove the whole statement if it says: "+=0;", "-=0;", "*=1;" or "/=1;"
             if (Token::Match(tok, "+=|-= 0 ;") ||
-                Token::Match(tok, "+=|-= '\\0' ;") ||
                 Token::simpleMatch(tok, "|= 0 ;") ||
                 Token::Match(tok, "*=|/= 1 ;")) {
                 tok = tok1;

test/testother.cpp

@@ -165,6 +165,7 @@ private:
 
         TEST_CASE(alwaysTrueFalseStringCompare);
         TEST_CASE(suspiciousStringCompare);
+        TEST_CASE(suspiciousStringCompare_char);
         TEST_CASE(checkSignOfUnsignedVariable);
         TEST_CASE(checkSignOfPointer);
 
@@ -5231,6 +5232,33 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void suspiciousStringCompare_char() {
+        check("bool foo(char* c) {\n"
+              "    return c == '\\0';\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str());
+
+        check("bool foo(char* c) {\n"
+              "    return '\\0' != c;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", errout.str());
+
+        check("bool foo(char c) {\n"
+              "    return c == '\\0';\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("bool foo(char c) {\n"
+              "    return c == 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* c) {\n"
+              "    if(c == '\\0') bar();\n"
+              "}");
+        TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) Char literal compared with pointer 'c'. Did you intend to dereference it?\n", "", errout.str());
+    }
+
     void check_signOfUnsignedVariable(const char code[], bool inconclusive=false) {
         // Clear the error buffer..
         errout.str("");