From c218859418170cd41f9cb30f4ec51838dad00f75 Mon Sep 17 00:00:00 2001
From: chrchr-github <78114321+chrchr-github@users.noreply.github.com>
Date: Sun, 10 Jul 2022 11:38:01 +0200
Subject: [PATCH] Fix some FNs related to c_str() (#4258)

---
 lib/checkstl.cpp | 27 +++++++++++++++++++++++++++
 lib/checkstl.h   |  3 +++
 test/teststl.cpp | 28 ++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp
index e65743f23..845bf5a8a 100644
--- a/lib/checkstl.cpp
+++ b/lib/checkstl.cpp
@@ -1953,6 +1953,9 @@ void CheckStl::string_c_str()
                     const Variable* var = tok->variable();
                     if (var->isPointer())
                         string_c_strError(tok);
+                } else if (printPerformance && Token::Match(tok->tokAt(2), "%var% . c_str|data ( ) ;")) {
+                    if (tok->variable()->isStlStringType() && tok->tokAt(2)->variable()->isStlStringType())
+                        string_c_strAssignment(tok);
                 }
             } else if (printPerformance && tok->function() && Token::Match(tok, "%name% ( !!)") && tok->str() != scope.className) {
                 const std::pair<std::multimap<const Function*, int>::const_iterator, std::multimap<const Function*, int>::const_iterator> range = c_strFuncParam.equal_range(tok->function());
@@ -1986,6 +1989,12 @@ void CheckStl::string_c_str()
 
                     }
                 }
+            } else if (printPerformance && Token::Match(tok, "%var% (|{ %var% . c_str|data ( )") && tok->variable()->isStlStringType() && tok->tokAt(2)->variable()->isStlStringType()) {
+                string_c_strConstructor(tok);
+            } else if (printPerformance && tok->next() && tok->next()->variable() && tok->next()->variable()->isStlStringType() && tok->valueType() && tok->valueType()->type == ValueType::CONTAINER &&
+                       ((Token::Match(tok->previous(), "%var% + %var% . c_str|data ( )") && tok->previous()->variable()->isStlStringType()) ||
+                        (Token::Match(tok->tokAt(-5), "%var% . c_str|data ( ) + %var%") && tok->tokAt(-5)->variable()->isStlStringType()))) {
+                string_c_strConcat(tok);
             }
 
             // Using c_str() to get the return value is only dangerous if the function returns a char*
@@ -2093,6 +2102,24 @@ void CheckStl::string_c_strParam(const Token* tok, nonneg int number)
     reportError(tok, Severity::performance, "stlcstrParam", oss.str(), CWE704, Certainty::normal);
 }
 
+void CheckStl::string_c_strConstructor(const Token* tok)
+{
+    std::string msg = "Constructing a std::string from the result of c_str() is slow and redundant.\nSolve that by directly passing the string.";
+    reportError(tok, Severity::performance, "stlcstrConstructor", msg, CWE704, Certainty::normal);
+}
+
+void CheckStl::string_c_strAssignment(const Token* tok)
+{
+    std::string msg = "Assigning the result of c_str() to a std::string is slow and redundant.\nSolve that by directly assigning the string.";
+    reportError(tok, Severity::performance, "stlcstrAssignment", msg, CWE704, Certainty::normal);
+}
+
+void CheckStl::string_c_strConcat(const Token* tok)
+{
+    std::string msg = "Concatenating the result of c_str() and a std::string is slow and redundant.\nSolve that by directly concatenating the strings.";
+    reportError(tok, Severity::performance, "stlcstrConcat", msg, CWE704, Certainty::normal);
+}
+
 //---------------------------------------------------------------------------
 //
 //---------------------------------------------------------------------------
diff --git a/lib/checkstl.h b/lib/checkstl.h
index 2bb7b80a5..3c0e2dc90 100644
--- a/lib/checkstl.h
+++ b/lib/checkstl.h
@@ -193,6 +193,9 @@ private:
     void string_c_strError(const Token* tok);
     void string_c_strReturn(const Token* tok);
     void string_c_strParam(const Token* tok, nonneg int number);
+    void string_c_strConstructor(const Token* tok);
+    void string_c_strAssignment(const Token* tok);
+    void string_c_strConcat(const Token* tok);
 
     void outOfBoundsError(const Token *tok, const std::string &containerName, const ValueFlow::Value *containerSize, const std::string &index, const ValueFlow::Value *indexValue);
     void outOfBoundsIndexExpressionError(const Token *tok, const Token *index);
diff --git a/test/teststl.cpp b/test/teststl.cpp
index 0caa695a0..0169118cd 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -4015,6 +4015,34 @@ private:
               "    }\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("std::string f(const std::string& a) {\n"
+              "    std::string b(a.c_str());\n"
+              "    return b;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (performance) Constructing a std::string from the result of c_str() is slow and redundant.\n", errout.str());
+
+        check("std::string f(const std::string& a) {\n"
+              "    std::string b{ a.c_str() };\n"
+              "    return b;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (performance) Constructing a std::string from the result of c_str() is slow and redundant.\n", errout.str());
+
+        check("std::string f(const std::string& a) {\n"
+              "    std::string b = a.c_str();\n"
+              "    return b;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (performance) Assigning the result of c_str() to a std::string is slow and redundant.\n", errout.str());
+
+        check("std::string g(const std::string& a, const std::string& b) {\n"
+              "    return a + b.c_str();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (performance) Concatenating the result of c_str() and a std::string is slow and redundant.\n", errout.str());
+
+        check("std::string g(const std::string& a, const std::string& b) {\n"
+              "    return a.c_str() + b;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (performance) Concatenating the result of c_str() and a std::string is slow and redundant.\n", errout.str());
     }
 
     void uselessCalls() {