From 423d7dbc3c7c000e0f16970de189e828e22ea63f Mon Sep 17 00:00:00 2001
From: PKEuS <philipp@pkeus.de>
Date: Sat, 20 Feb 2021 12:42:48 +0100
Subject: [PATCH] Fixed false negatives literalWithCharPtrCompare when
 address-of operator (C only) or arrays are used, adapted TODO unit tests
 Enabled working unit test in testunusedvar.cpp

Merged from LCppC.
---
 lib/checkstring.cpp    | 19 +++++++++++--------
 test/teststring.cpp    | 17 +++++++++--------
 test/testunusedvar.cpp |  5 ++---
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/lib/checkstring.cpp b/lib/checkstring.cpp
index 576be7a1b..709c6f9f5 100644
--- a/lib/checkstring.cpp
+++ b/lib/checkstring.cpp
@@ -195,12 +195,19 @@ void CheckString::checkSuspiciousStringCompare()
                 }
             }
 
-            if (varTok->str() == "*") {
+            const Token* oldVarTok = varTok;
+            while (varTok->str() == "*") {
                 if (!mTokenizer->isC() || varTok->astOperand2() != nullptr || litTok->tokType() != Token::eString)
-                    continue;
+                    break;
                 varTok = varTok->astOperand1();
             }
 
+            if (mTokenizer->isC() && varTok->str() == "&")
+                varTok = varTok->astOperand1();
+
+            if (varTok->str() == "[")
+                varTok = varTok->astOperand1();
+
             while (varTok && (varTok->str() == "." || varTok->str() == "::"))
                 varTok = varTok->astOperand2();
             if (!varTok || !varTok->isName())
@@ -208,16 +215,12 @@ void CheckString::checkSuspiciousStringCompare()
 
             const Variable *var = varTok->variable();
 
-            while (Token::Match(varTok->astParent(), "[.*]"))
-                varTok = varTok->astParent();
-            const std::string varname = varTok->expressionString();
-
             const bool ischar(litTok->tokType() == Token::eChar);
             if (litTok->tokType() == Token::eString) {
                 if (mTokenizer->isC() || (var && var->isArrayOrPointer()))
-                    suspiciousStringCompareError(tok, varname, litTok->isLong());
+                    suspiciousStringCompareError(tok, oldVarTok->expressionString(), litTok->isLong());
             } else if (ischar && var && var->isPointer()) {
-                suspiciousStringCompareError_char(tok, varname);
+                suspiciousStringCompareError_char(tok, oldVarTok->expressionString());
             }
         }
     }
diff --git a/test/teststring.cpp b/test/teststring.cpp
index f6d74cbfc..17a3d457a 100644
--- a/test/teststring.cpp
+++ b/test/teststring.cpp
@@ -289,6 +289,11 @@ private:
               "}");
         ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n", errout.str());
 
+        check("bool foo(char** c) {\n"
+              "    return c[3] == \"x\";\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c[3]'. Did you intend to use strcmp() instead?\n", errout.str());
+
         check("bool foo(wchar_t* c) {\n"
               "    return c == L\"x\";\n"
               "}");
@@ -349,18 +354,14 @@ private:
         // Ticket #4257
         check("bool foo() {\n"
               "MyString **str=OtherGetter();\n"
-              "return *str==\"bug\"; }");
-        TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n",
-                           "",
-                           errout.str());
+              "return *str==\"bug\"; }", "test.c");
+        ASSERT_EQUALS("[test.c:3]: (warning) String literal compared with variable '*str'. Did you intend to use strcmp() instead?\n", errout.str());
 
         // Ticket #4257
         check("bool foo() {\n"
               "MyString str=OtherGetter2();\n"
-              "return &str==\"bug\"; }");
-        TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) String literal compared with variable 'c'. Did you intend to use strcmp() instead?\n",
-                           "",
-                           errout.str());
+              "return &str==\"bug\"; }", "test.c");
+        ASSERT_EQUALS("[test.c:3]: (warning) String literal compared with variable '&str'. Did you intend to use strcmp() instead?\n", errout.str());
 
         // Ticket #5734
         check("int foo(char c) {\n"
diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp
index c485e3a6d..d320577b4 100644
--- a/test/testunusedvar.cpp
+++ b/test/testunusedvar.cpp
@@ -5737,9 +5737,8 @@ private:
         functionVariableUsage(
             "void fun(std::string s) {\n"
             " s[10] = 123;\n"
-            "}\n"
-        );
-        // TODO This works on command line.. load std.cfg? ASSERT_EQUALS("error", errout.str());
+            "}");
+        ASSERT_EQUALS("[test.cpp:2]: (style) Variable 's[10]' is assigned a value that is never used.\n", errout.str());
 
         functionVariableUsage(
             "void fun(short data[2]) {\n"