try to clarify error message

lib/checkstring.cpp

@@ -336,7 +336,7 @@ void CheckString::incorrectStringBooleanError(const Token *tok, const std::strin
 // always true: strcmp(str,"a")==0 || strcmp(str,"b")
 // TODO: Library configuration for string comparison functions
 //---------------------------------------------------------------------------
-void CheckString::overlappingStringComparisons()
+void CheckString::deadStrcmp()
 {
     if (!_settings->isEnabled(Settings::WARNING))
         return;
@@ -399,14 +399,14 @@ void CheckString::overlappingStringComparisons()
                         args2[1]->isLiteral() &&
                         args1[1]->str() != args2[1]->str() &&
                         isSameExpression(_tokenizer->isCPP(), true, args1[0], args2[0], _settings->library, true))
-                        overlappingStringComparisonsError(tok1, tok2);
+                        deadStrcmpError(tok1, tok2);
                 }
             }
         }
     }
 }
 
-void CheckString::overlappingStringComparisonsError(const Token *eq0, const Token *ne0)
+void CheckString::deadStrcmpError(const Token *eq0, const Token *ne0)
 {
     std::string eq0Expr(eq0 ? eq0->expressionString() : std::string("strcmp(x,\"abc\")"));
     if (eq0 && eq0->astParent()->str() == "!")
@@ -414,7 +414,14 @@ void CheckString::overlappingStringComparisonsError(const Token *eq0, const Toke
     else
         eq0Expr += " == 0";
     const std::string ne0Expr = (ne0 ? ne0->expressionString() : std::string("strcmp(x,\"def\")")) + " != 0";
-    reportError(ne0, Severity::warning, "overlappingStringComparisons", "The comparison operator in '" + ne0Expr + "' should maybe be '==' instead, currently the expression '" + eq0Expr + "' is redundant.");
+
+    const Token *ortok = eq0;
+    while (ortok && ortok->str() != "||")
+        ortok = ortok->astParent();
+
+    const std::string expression = ortok ? ortok->expressionString() : (eq0Expr + " || " + ne0Expr);
+
+    reportError(ne0, Severity::warning, "deadStrcmp", "The expression '" + expression + "' has a dead string comparison. The expression is logically the same as \'" + ne0Expr + "\'.");
 }
 
 //---------------------------------------------------------------------------

lib/checkstring.h

@@ -57,7 +57,7 @@ public:
         checkString.strPlusChar();
         checkString.checkSuspiciousStringCompare();
         checkString.stringLiteralWrite();
-        checkString.overlappingStringComparisons();
+        checkString.deadStrcmp();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -85,8 +85,8 @@ public:
     /** @brief %Check for suspicious code that compares string literals for equality */
     void checkAlwaysTrueOrFalseStringCompare();
 
-    /** @brief %Check for overlapping string comparisons */
-    void overlappingStringComparisons();
+    /** @brief %Check for dead string comparison */
+    void deadStrcmp();
 
     /** @brief %Check for overlapping source and destination passed to sprintf() */
     void sprintfOverlappingData();
@@ -101,7 +101,7 @@ private:
     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 overlappingStringComparisonsError(const Token* tok1, const Token *tok2);
+    void deadStrcmpError(const Token* tok1, const Token *tok2);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckString c(nullptr, settings, errorLogger);
@@ -115,7 +115,7 @@ private:
         c.incorrectStringBooleanError(nullptr, "\"Hello World\"");
         c.alwaysTrueFalseStringCompareError(nullptr, "str1", "str2");
         c.alwaysTrueStringVariableCompareError(nullptr, "varname1", "varname2");
-        c.overlappingStringComparisonsError(nullptr, nullptr);
+        c.deadStrcmpError(nullptr, nullptr);
     }
 
     static std::string myName() {
@@ -130,7 +130,7 @@ private:
                "- 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"
-               "- overlapping string comparisons (strcmp(str,\"x\")==0 || strcmp(str,\"y\")!=0)\n";
+               "- dead string comparison; (strcmp(str,\"x\")==0 || strcmp(str,\"y\")) is logically the same as (strcmp(str,\"y\") != 0)\n";
     }
 };
 /// @}

test/teststring.cpp

@@ -52,7 +52,7 @@ private:
 
         TEST_CASE(incorrectStringCompare);
 
-        TEST_CASE(overlappingStringComparisons);
+        TEST_CASE(deadStrcmp);
     }
 
     void check(const char code[], const char filename[] = "test.cpp") {
@@ -581,11 +581,11 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
-    void overlappingStringComparisons() {
+    void deadStrcmp() {
         check("void f(const char *str) {\n"
               "  if (strcmp(str, \"abc\") == 0 || strcmp(str, \"def\")) {}\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:2]: (warning) The comparison operator in 'strcmp(str,\"def\") != 0' should maybe be '==' instead, currently the expression 'strcmp(str,\"abc\") == 0' is redundant.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:2]: (warning) The expression 'strcmp(str,\"abc\")==0||strcmp(str,\"def\")' has a dead string comparison. The expression is logically the same as 'strcmp(str,\"def\") != 0'.\n", errout.str());
 
         check("struct X {\n"
               "  char *str;\n"
@@ -594,7 +594,7 @@ private:
               "void f(const struct X *x) {\n"
               "  if (strcmp(x->str, \"abc\") == 0 || strcmp(x->str, \"def\")) {}\n"
               "}");
-        ASSERT_EQUALS("[test.cpp:6]: (warning) The comparison operator in 'strcmp(x->str,\"def\") != 0' should maybe be '==' instead, currently the expression 'strcmp(x->str,\"abc\") == 0' is redundant.\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:6]: (warning) The expression 'strcmp(x->str,\"abc\")==0||strcmp(x->str,\"def\")' has a dead string comparison. The expression is logically the same as 'strcmp(x->str,\"def\") != 0'.\n", errout.str());
     }
 };