Refactorizations optimizing std::string usage:

PKEuS · PKEuS · commit ec1bd420a771 · 2014-06-26T11:51:02.000+02:00
1) Added global static const std::string emptyString; object:
-&gt; Replaces some static variables in functions which might be not threadsafe
-&gt; Avoids constructor call (std::string::string(""))
-&gt; Even functions that return an empty string in some branches can return by reference now.
Added to config.h to ensure that it is available everywhere

2) Added overloads for TestFixture::assertEquals for the most common use cases:
-&gt; Moves conversion from const char[] to std::string into a function, reducing code duplication in binary.
diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
@@ -1069,7 +1069,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str
             (declarationId == 0 && Token::Match(tok, ("strcpy|strcat ( " + varnames + " , %str% )").c_str()))) {
             const std::size_t len = Token::getStrLength(tok->tokAt(varcount + 4));
             if (total_size > 0 && len >= (unsigned int)total_size) {
-                bufferOverrunError(tok, declarationId > 0 ? std::string() : varnames);
+                bufferOverrunError(tok, declarationId > 0 ? emptyString : varnames);
                 continue;
             }
         } else if ((declarationId > 0 && Token::Match(tok, "strcpy|strcat ( %varid% , %var% )", declarationId)) ||
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
@@ -210,8 +210,8 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
 
     static bool isArrayOfStruct(const Token* tok, int &position);
     void arrayIndexOutOfBoundsError(const std::list<const Token *> &callstack, const ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index);
-    void bufferOverrunError(const Token *tok, const std::string &varnames = "");
-    void bufferOverrunError(const std::list<const Token *> &callstack, const std::string &varnames = "");
+    void bufferOverrunError(const Token *tok, const std::string &varnames = emptyString);
+    void bufferOverrunError(const std::list<const Token *> &callstack, const std::string &varnames = emptyString);
     void strncatUsageError(const Token *tok);
     void negativeMemoryAllocationSizeError(const Token *tok); // provide a negative value to memory allocation function
     void outOfBoundsError(const Token *tok, const std::string &what, const bool show_size_info, const MathLib::bigint &supplied_size, const MathLib::bigint &actual_size);
diff --git a/lib/config.h b/lib/config.h
@@ -19,4 +19,7 @@
 #  include <crtdbg.h>
 #endif
 
+#include <string>
+static const std::string emptyString;
+
 #endif // configH
diff --git a/lib/errorlogger.h b/lib/errorlogger.h
@@ -217,8 +217,8 @@ class CPPCHECKLIB ErrorLogger {
          * @param outputFormat Empty string to use default output format
          * or template to be used. E.g. "{file}:{line},{severity},{id},{message}"
         * @return formatted string
-               */
-        std::string toString(bool verbose, const std::string &outputFormat = "") const;
+         */
+        std::string toString(bool verbose, const std::string &outputFormat = emptyString) const;
 
         std::string serialize() const;
         bool deserialize(const std::string &data);
diff --git a/lib/library.h b/lib/library.h
@@ -184,9 +184,9 @@ class CPPCHECKLIB Library {
 
     bool isargvalid(const std::string &functionName, int argnr, const MathLib::bigint argvalue) const;
 
-    std::string validarg(const std::string &functionName, int argnr) const {
+    const std::string& validarg(const std::string &functionName, int argnr) const {
         const ArgumentChecks *arg = getarg(functionName, argnr);
-        return arg ? arg->valid : std::string("");
+        return arg ? arg->valid : emptyString;
     }
 
     bool markupFile(const std::string &path) const {
@@ -227,24 +227,24 @@ class CPPCHECKLIB Library {
         return offset;
     }
 
-    std::string blockstart(const std::string &file) const {
+    const std::string& blockstart(const std::string &file) const {
         const std::map<std::string, CodeBlock>::const_iterator map_it
             = _executableblocks.find(Path::getFilenameExtensionInLowerCase(file));
 
         if (map_it != _executableblocks.end()) {
             return map_it->second.start();
         }
-        return std::string();
+        return emptyString;
     }
 
-    std::string blockend(const std::string &file) const {
+    const std::string& blockend(const std::string &file) const {
         const std::map<std::string, CodeBlock>::const_iterator map_it
             = _executableblocks.find(Path::getFilenameExtensionInLowerCase(file));
 
         if (map_it != _executableblocks.end()) {
             return map_it->second.end();
         }
-        return std::string();
+        return emptyString;
     }
 
     bool iskeyword(const std::string &file, const std::string &keyword) const {
diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp
@@ -1040,7 +1040,7 @@ void Preprocessor::preprocess(std::istream &srcCodeStream, std::string &processe
         processedFile = ostr.str();
     }
 
-    std::map<std::string, std::string> defs(getcfgmap(_settings ? _settings->userDefines : std::string(""), _settings, filename));
+    std::map<std::string, std::string> defs(getcfgmap(_settings ? _settings->userDefines : emptyString, _settings, filename));
 
     if (_settings && _settings->_maxConfigs == 1U) {
         std::set<std::string> pragmaOnce;
diff --git a/lib/suppressions.h b/lib/suppressions.h
@@ -99,7 +99,7 @@ class CPPCHECKLIB Suppressions {
      * @param line number, e.g. "123"
      * @return error message. empty upon success
      */
-    std::string addSuppression(const std::string &errorId, const std::string &file = "", unsigned int line = 0);
+    std::string addSuppression(const std::string &errorId, const std::string &file = emptyString, unsigned int line = 0);
 
     /**
      * @brief Returns true if this message should not be shown to the user.
diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h
@@ -104,8 +104,7 @@ class CPPCHECKLIB Type {
     }
 
     const std::string& name() const {
-        static const std::string empty;
-        return classDef->next()->isName() ? classDef->strAt(1) : empty;
+        return classDef->next()->isName() ? classDef->strAt(1) : emptyString;
     }
 
     const Token *initBaseInfo(const Token *tok, const Token *tok1);
@@ -215,13 +214,11 @@ class CPPCHECKLIB Variable {
      * @return name string
      */
     const std::string &name() const {
-        static const std::string noname;
-
         // name may not exist for function arguments
         if (_name)
             return _name->str();
 
-        return noname;
+        return emptyString;
     }
 
     /**
diff --git a/lib/token.cpp b/lib/token.cpp
@@ -266,10 +266,8 @@ const Token *Token::linkAt(int index) const
 
 const std::string &Token::strAt(int index) const
 {
-    static const std::string empty_str;
-
     const Token *tok = this->tokAt(index);
-    return tok ? tok->_str : empty_str;
+    return tok ? tok->_str : emptyString;
 }
 
 static int multiComparePercent(const Token *tok, const char ** haystack_p,
diff --git a/lib/tokenize.h b/lib/tokenize.h
@@ -90,7 +90,7 @@ class CPPCHECKLIB Tokenizer {
      */
     bool tokenize(std::istream &code,
                   const char FileName[],
-                  const std::string &configuration = "",
+                  const std::string &configuration = emptyString,
                   bool noSymbolDB_AST = false);
     /**
      * tokenize condition and run simple simplifications on it
diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp
@@ -56,8 +56,7 @@ TokenList::~TokenList()
 const std::string& TokenList::getSourceFilePath() const
 {
     if (getFiles().empty()) {
-        static const std::string empty;
-        return empty;
+        return emptyString;
     }
     return getFiles()[0];
 }
diff --git a/lib/tokenlist.h b/lib/tokenlist.h
@@ -73,7 +73,7 @@ class CPPCHECKLIB TokenList {
      * @param code input stream for code
      * @param file0 source file name
      */
-    bool createTokens(std::istream &code, const std::string& file0 = "");
+    bool createTokens(std::istream &code, const std::string& file0 = emptyString);
 
     /** Deallocate list */
     void deallocateTokens();
diff --git a/test/testsuite.cpp b/test/testsuite.cpp
@@ -149,6 +149,18 @@ void TestFixture::assertEquals(const char *filename, unsigned int linenr, const
         }
     }
 }
+void TestFixture::assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg) const
+{
+    assertEquals(filename, linenr, std::string(expected), actual, msg);
+}
+void TestFixture::assertEquals(const char *filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg) const
+{
+    assertEquals(filename, linenr, std::string(expected), std::string(actual), msg);
+}
+void TestFixture::assertEquals(const char *filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg) const
+{
+    assertEquals(filename, linenr, expected, std::string(actual), msg);
+}
 
 void TestFixture::assertEquals(const char *filename, unsigned int linenr, long long expected, long long actual, const std::string &msg) const
 {
diff --git a/test/testsuite.h b/test/testsuite.h
@@ -52,9 +52,12 @@ class TestFixture : public ErrorLogger {
     void assert_(const char *filename, unsigned int linenr, bool condition) const;
     void todoAssert(const char *filename, unsigned int linenr, bool condition) const;
 
-    void assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = "") const;
-    void assertEquals(const char *filename, unsigned int linenr, long long expected, long long actual, const std::string &msg="") const;
-    void assertEqualsDouble(const char *filename, unsigned int linenr, double expected, double actual, const std::string &msg="") const;
+    void assertEquals(const char *filename, unsigned int linenr, const std::string &expected, const std::string &actual, const std::string &msg = emptyString) const;
+    void assertEquals(const char *filename, unsigned int linenr, const char expected[], const std::string& actual, const std::string &msg = emptyString) const;
+    void assertEquals(const char *filename, unsigned int linenr, const char expected[], const char actual[], const std::string &msg = emptyString) const;
+    void assertEquals(const char *filename, unsigned int linenr, const std::string& expected, const char actual[], const std::string &msg = emptyString) const;
+    void assertEquals(const char *filename, unsigned int linenr, long long expected, long long actual, const std::string &msg = emptyString) const;
+    void assertEqualsDouble(const char *filename, unsigned int linenr, double expected, double actual, const std::string &msg = emptyString) const;
 
     void todoAssertEquals(const char *filename, unsigned int linenr, const std::string &wanted,
                           const std::string &current, const std::string &actual) const;
diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp
@@ -115,7 +115,7 @@ class TestSuppressions : public TestFixture {
     }
 
     // Check the suppression
-    void checkSuppression(const char code[], const std::string &suppression = "") {
+    void checkSuppression(const char code[], const std::string &suppression = emptyString) {
         // Clear the error log
         errout.str("");
 
@@ -133,7 +133,7 @@ class TestSuppressions : public TestFixture {
         reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions());
     }
 
-    void checkSuppressionThreads(const char code[], const std::string &suppression = "") {
+    void checkSuppressionThreads(const char code[], const std::string &suppression = emptyString) {
         errout.str("");
         output.str("");
 
@@ -157,7 +157,7 @@ class TestSuppressions : public TestFixture {
     }
 
     // Check the suppression for multiple files
-    void checkSuppression(const char *names[], const char *codes[], const std::string &suppression = "") {
+    void checkSuppression(const char *names[], const char *codes[], const std::string &suppression = emptyString) {
         // Clear the error log
         errout.str("");
 

Original file line number	Diff line number	Diff line change
`@@ -1069,7 +1069,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const std::vector<std::str`
`1069`	`1069`	`(declarationId == 0 && Token::Match(tok, ("strcpy\|strcat ( " + varnames + " , %str% )").c_str()))) {`
`1070`	`1070`	`const std::size_t len = Token::getStrLength(tok->tokAt(varcount + 4));`
`1071`	`1071`	`if (total_size > 0 && len >= (unsigned int)total_size) {`
`1072`		`- bufferOverrunError(tok, declarationId > 0 ? std::string() : varnames);`
	`1072`	`+ bufferOverrunError(tok, declarationId > 0 ? emptyString : varnames);`
`1073`	`1073`	`continue;`
`1074`	`1074`	`}`
`1075`	`1075`	`} else if ((declarationId > 0 && Token::Match(tok, "strcpy\|strcat ( %varid% , %var% )", declarationId)) \|\|`
Original file line number	Diff line number	Diff line change
`@@ -184,9 +184,9 @@ class CPPCHECKLIB Library {`
`184`	`184`
`185`	`185`	`bool isargvalid(const std::string &functionName, int argnr, const MathLib::bigint argvalue) const;`
`186`	`186`
`187`		`- std::string validarg(const std::string &functionName, int argnr) const {`
	`187`	`+ const std::string& validarg(const std::string &functionName, int argnr) const {`
`188`	`188`	`const ArgumentChecks *arg = getarg(functionName, argnr);`
`189`		`- return arg ? arg->valid : std::string("");`
	`189`	`+ return arg ? arg->valid : emptyString;`
`190`	`190`	`}`
`191`	`191`
`192`	`192`	`bool markupFile(const std::string &path) const {`
`@@ -227,24 +227,24 @@ class CPPCHECKLIB Library {`
`227`	`227`	`return offset;`
`228`	`228`	`}`
`229`	`229`
`230`		`- std::string blockstart(const std::string &file) const {`
	`230`	`+ const std::string& blockstart(const std::string &file) const {`
`231`	`231`	`const std::map<std::string, CodeBlock>::const_iterator map_it`
`232`	`232`	`= _executableblocks.find(Path::getFilenameExtensionInLowerCase(file));`
`233`	`233`
`234`	`234`	`if (map_it != _executableblocks.end()) {`
`235`	`235`	`return map_it->second.start();`
`236`	`236`	`}`
`237`		`- return std::string();`
	`237`	`+ return emptyString;`
`238`	`238`	`}`
`239`	`239`
`240`		`- std::string blockend(const std::string &file) const {`
	`240`	`+ const std::string& blockend(const std::string &file) const {`
`241`	`241`	`const std::map<std::string, CodeBlock>::const_iterator map_it`
`242`	`242`	`= _executableblocks.find(Path::getFilenameExtensionInLowerCase(file));`
`243`	`243`
`244`	`244`	`if (map_it != _executableblocks.end()) {`
`245`	`245`	`return map_it->second.end();`
`246`	`246`	`}`
`247`		`- return std::string();`
	`247`	`+ return emptyString;`
`248`	`248`	`}`
`249`	`249`
`250`	`250`	`bool iskeyword(const std::string &file, const std::string &keyword) const {`
Original file line number	Diff line number	Diff line change
`@@ -1040,7 +1040,7 @@ void Preprocessor::preprocess(std::istream &srcCodeStream, std::string &processe`
`1040`	`1040`	`processedFile = ostr.str();`
`1041`	`1041`	`}`
`1042`	`1042`
`1043`		`- std::map<std::string, std::string> defs(getcfgmap(_settings ? _settings->userDefines : std::string(""), _settings, filename));`
	`1043`	`+ std::map<std::string, std::string> defs(getcfgmap(_settings ? _settings->userDefines : emptyString, _settings, filename));`
`1044`	`1044`
`1045`	`1045`	`if (_settings && _settings->_maxConfigs == 1U) {`
`1046`	`1046`	`std::set<std::string> pragmaOnce;`
Original file line number	Diff line number	Diff line change
`@@ -266,10 +266,8 @@ const Token *Token::linkAt(int index) const`
`266`	`266`
`267`	`267`	`const std::string &Token::strAt(int index) const`
`268`	`268`	`{`
`269`		`- static const std::string empty_str;`
`270`		`-`
`271`	`269`	`const Token *tok = this->tokAt(index);`
`272`		`- return tok ? tok->_str : empty_str;`
	`270`	`+ return tok ? tok->_str : emptyString;`
`273`	`271`	`}`
`274`	`272`
`275`	`273`	`static int multiComparePercent(const Token tok, const char * haystack_p,`