Fix #7515 New check: Not needed c_str() operation (#4371)

Author: chrchr-github

lib/astutils.cpp

@@ -380,6 +380,12 @@ bool isVariableDecl(const Token* tok)
     return false;
 }
 
+bool isStlStringType(const Token* tok)
+{
+    return Token::Match(tok, "std :: string|wstring|u16string|u32string !!::") ||
+           (Token::simpleMatch(tok, "std :: basic_string <") && !Token::simpleMatch(tok->linkAt(3), "> ::"));
+}
+
 bool isTemporary(bool cpp, const Token* tok, const Library* library, bool unknown)
 {
     if (!tok)

lib/astutils.h

@@ -162,6 +162,7 @@ std::string astCanonicalType(const Token *expr);
 const Token * astIsVariableComparison(const Token *tok, const std::string &comp, const std::string &rhs, const Token **vartok=nullptr);
 
 bool isVariableDecl(const Token* tok);
+bool isStlStringType(const Token* tok);
 
 bool isTemporary(bool cpp, const Token* tok, const Library* library, bool unknown = false);
 

lib/checkstl.cpp

@@ -2009,6 +2009,18 @@ void CheckStl::string_c_str()
                        ((Token::Match(tok->previous(), "%var% + %var% . c_str|data ( )") && tok->previous()->variable() && tok->previous()->variable()->isStlStringType()) ||
                         (Token::Match(tok->tokAt(-5), "%var% . c_str|data ( ) + %var%") && tok->tokAt(-5)->variable() && tok->tokAt(-5)->variable()->isStlStringType()))) {
                 string_c_strConcat(tok);
+            } else if (printPerformance && Token::simpleMatch(tok, "<<") && tok->astOperand2() && Token::simpleMatch(tok->astOperand2()->astOperand1(), ". c_str ( )")) {
+                const Token* str = tok->astOperand2()->astOperand1()->astOperand1();
+                if (Token::Match(str, "(|["))
+                    str = str->previous();
+                if (str && ((str->variable() && str->variable()->isStlStringType()) ||
+                            (str->function() && isStlStringType(str->function()->retDef)))) {
+                    const Token* strm = tok;
+                    while (Token::simpleMatch(strm, "<<"))
+                        strm = strm->astOperand1();
+                    if (strm && strm->variable() && strm->variable()->isStlType())
+                        string_c_strStream(tok);
+                }
             }
 
             // Using c_str() to get the return value is only dangerous if the function returns a char*
@@ -2118,22 +2130,32 @@ void CheckStl::string_c_strParam(const Token* tok, nonneg int number)
 
 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.";
+    std::string msg = "Constructing a std::string from the result of c_str() is slow and redundant.\n"
+                      "Constructing a std::string from const char* requires a call to strlen(). Solve 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.";
+    std::string msg = "Assigning the result of c_str() to a std::string is slow and redundant.\n"
+                      "Assigning a const char* to a std::string requires a call to strlen(). Solve 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.";
+    std::string msg = "Concatenating the result of c_str() and a std::string is slow and redundant.\n"
+                      "Concatenating a const char* with a std::string requires a call to strlen(). Solve that by directly concatenating the strings.";
     reportError(tok, Severity::performance, "stlcstrConcat", msg, CWE704, Certainty::normal);
 }
 
+void CheckStl::string_c_strStream(const Token* tok)
+{
+    std::string msg = "Passing the result of c_str() to a stream is slow and redundant.\n"
+                      "Passing a const char* to a stream requires a call to strlen(). Solve that by directly passing the string.";
+    reportError(tok, Severity::performance, "stlcstrStream", msg, CWE704, Certainty::normal);
+}
+
 //---------------------------------------------------------------------------
 //
 //---------------------------------------------------------------------------

lib/checkstl.h

@@ -196,6 +196,7 @@ class CPPCHECKLIB CheckStl : public Check {
     void string_c_strConstructor(const Token* tok);
     void string_c_strAssignment(const Token* tok);
     void string_c_strConcat(const Token* tok);
+    void string_c_strStream(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);

lib/symboldatabase.cpp

@@ -2187,7 +2187,7 @@ void Variable::evaluate(const Settings* settings)
             strtype += "::" + typeToken->strAt(2);
         setFlag(fIsClass, !lib->podtype(strtype) && !mTypeStartToken->isStandardType() && !isEnumType() && !isPointer() && !isReference() && strtype != "...");
         setFlag(fIsStlType, Token::simpleMatch(mTypeStartToken, "std ::"));
-        setFlag(fIsStlString, isStlType() && (Token::Match(mTypeStartToken->tokAt(2), "string|wstring|u16string|u32string !!::") || (Token::simpleMatch(mTypeStartToken->tokAt(2), "basic_string <") && !Token::simpleMatch(mTypeStartToken->linkAt(3), "> ::"))));
+        setFlag(fIsStlString, ::isStlStringType(mTypeStartToken));
         setFlag(fIsSmartPointer, lib->isSmartPointer(mTypeStartToken));
     }
     if (mAccess == AccessControl::Argument) {

test/teststl.cpp

@@ -4067,6 +4067,21 @@ class TestStl : public TestFixture {
               "    const double* const QM_R__ buf(v.data() + i);\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        check("struct T { std::string g(); std::string a[1]; }\n" // #7515
+              "void f(std::stringstream& strm, const std::string& s, T& t) {\n"
+              "    strm << s.c_str();\n"
+              "    strm << \"abc\" << s.c_str();\n"
+              "    strm << \"abc\" << s.c_str() << \"def\";\n"
+              "    strm << \"abc\" << t.g().c_str() << \"def\";\n"
+              "    strm << t.a[0].c_str();\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n"
+                      "[test.cpp:4]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n"
+                      "[test.cpp:5]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n"
+                      "[test.cpp:6]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n"
+                      "[test.cpp:7]: (performance) Passing the result of c_str() to a stream is slow and redundant.\n",
+                      errout.str());
     }
 
     void uselessCalls() {