Refactorizations:

- Increased encapsulation by making some functions private
- Removed redundant function CheckBufferOverrun::ArrayInfo::declare
- Avoided copy of ArrayInfo object
- Removed unnecessary and suspicious "if(sizeof(int) == 4)"

lib/checkbufferoverrun.cpp

@@ -1239,7 +1239,7 @@ void CheckBufferOverrun::checkGlobalAndLocalVariable()
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
         if (Token::Match(tok, "%str% [ %num% ]")) {
             std::string str = tok->strValue();
-            std::size_t index = std::atoi(tok->tokAt(2)->str().c_str());
+            std::size_t index = std::atoi(tok->strAt(2).c_str());
             if (index > str.length()) {
                 bufferOverrunError(tok, tok->str());
             }
@@ -1892,70 +1892,6 @@ CheckBufferOverrun::ArrayInfo CheckBufferOverrun::ArrayInfo::limit(MathLib::bigi
     return ArrayInfo(_varid, _varname, _element_size, n - uvalue);
 }
 
-bool CheckBufferOverrun::ArrayInfo::declare(const Token *tok, const Tokenizer &tokenizer)
-{
-    _num.clear();
-    _element_size = 0;
-    _varname.clear();
-
-    if (!tok)
-        return false;
-
-    if (!tok->isName() || tok->str() == "return")
-        return false;
-
-    while (tok && (tok->str() == "static" ||
-                   tok->str() == "const" ||
-                   tok->str() == "extern"))
-        tok = tok->next();
-
-    // ivar : number of type tokens
-    int ivar = 0;
-    if (Token::Match(tok, "%type% *| %var% ["))
-        ivar = 1;
-    else if (Token::Match(tok, "%type% %type% *| %var% ["))
-        ivar = 2;
-    else
-        return false;
-
-    if (tok->str().find(":") != std::string::npos)
-        return false;
-
-    // Goto variable name token, get element size..
-    const Token *vartok = tok->tokAt(ivar);
-    if (vartok->str() == "*") {
-        _element_size = tokenizer.sizeOfType(vartok);
-        vartok = vartok->next();
-    } else if (tok->str() == "struct") {
-        _element_size = 100;
-    } else {
-        _element_size = tokenizer.sizeOfType(tok);
-    }
-
-    _varname = vartok->str();
-    _varid = vartok->varId();
-    if (!_varid)
-        return false;
-
-    const Token *atok = vartok->tokAt(2);
-
-    if (!Token::Match(atok, "%num% ] ;|=|["))
-        return false;
-
-    while (Token::Match(atok, "%num% ] ;|=|[")) {
-        _num.push_back(MathLib::toLongNumber(atok->str()));
-        atok = atok->next();
-        if (Token::simpleMatch(atok, "] ["))
-            atok = atok->tokAt(2);
-    }
-
-    if (Token::Match(atok, "] = !!{"))
-        return false;
-
-    return (!_num.empty() && Token::Match(atok, "] ;|="));
-}
-
-
 
 /**
  * @brief %Check for buffer overruns (using ExecutionPath)
@@ -2028,20 +1964,17 @@ private:
             return;
 
         // Locate array info corresponding to varid1
-        CheckBufferOverrun::ArrayInfo ai;
+        ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(checks.front());
-        {
+        std::map<unsigned int, CheckBufferOverrun::ArrayInfo>::const_iterator it1;
-            ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(checks.front());
+        it1 = c->arrayInfo.find(varid1);
-            std::map<unsigned int, CheckBufferOverrun::ArrayInfo>::const_iterator it;
+        if (it1 == c->arrayInfo.end())
-            it = c->arrayInfo.find(varid1);
+            return;
-            if (it == c->arrayInfo.end())
+        const CheckBufferOverrun::ArrayInfo& ai = it1->second;
-                return;
-            ai = it->second;
-        }
 
         // Check if varid2 variable has a value that is out-of-bounds
         std::list<ExecutionPath *>::const_iterator it;
         for (it = checks.begin(); it != checks.end(); ++it) {
-            ExecutionPathBufferOverrun *c = dynamic_cast<ExecutionPathBufferOverrun *>(*it);
+            c = dynamic_cast<ExecutionPathBufferOverrun *>(*it);
             if (c && c->varId == varid2 && c->value >= ai.num(0)) {
                 // variable value is out of bounds, report error
                 CheckBufferOverrun *checkBufferOverrun = dynamic_cast<CheckBufferOverrun *>(c->owner);

lib/checkbufferoverrun.h

@@ -140,14 +140,6 @@ public:
         /** Create a copy ArrayInfo where the number of elements have been limited by a value */
         ArrayInfo limit(MathLib::bigint value) const;
 
-        /**
-         * Declare array - set info
-         * \param tok first token in array declaration
-         * \param tokenizer The tokenizer (for type size)
-         * \return success => true
-         */
-        bool declare(const Token *tok, const Tokenizer &tokenizer);
-
         /** array sizes */
         const std::vector<MathLib::bigint> &num() const {
             return _num;

lib/checkstl.h

@@ -83,13 +83,6 @@ public:
      */
     void mismatchingContainers();
 
-    /**
-     * Dereferencing an erased iterator
-     * @param tok token where error occurs
-     * @param itername iterator name
-     */
-    void dereferenceErasedError(const Token *tok, const std::string &itername);
-
     /**
      * Dangerous usage of erase. The iterator is invalidated by erase so
      * it is bad to dereference it after the erase.
@@ -129,14 +122,9 @@ public:
      *  - may unintentionally skip elements in list/set etc
      */
     void missingComparison();
-    void missingComparisonError(const Token *incrementToken1, const Token *incrementToken2);
 
     /** Check for common mistakes when using the function string::c_str() */
     void string_c_str();
-    void string_c_strThrowError(const Token *tok);
-    void string_c_strError(const Token *tok);
-    void string_c_strReturn(const Token *tok);
-    void string_c_strParam(const Token *tok, unsigned int number);
 
     /** @brief %Check for use and copy auto pointer */
     void checkAutoPointer();
@@ -155,6 +143,19 @@ private:
      */
     void eraseCheckLoop(const Token *it);
 
+    /**
+     * Dereferencing an erased iterator
+     * @param tok token where error occurs
+     * @param itername iterator name
+     */
+    void dereferenceErasedError(const Token *tok, const std::string &itername);
+
+    void missingComparisonError(const Token *incrementToken1, const Token *incrementToken2);
+    void string_c_strThrowError(const Token *tok);
+    void string_c_strError(const Token *tok);
+    void string_c_strReturn(const Token *tok);
+    void string_c_strParam(const Token *tok, unsigned int number);
+
     void stlOutOfBoundsError(const Token *tok, const std::string &num, const std::string &var, bool at);
     void invalidIteratorError(const Token *tok, const std::string &iteratorName);
     void iteratorsError(const Token *tok, const std::string &container1, const std::string &container2);

lib/checkuninitvar.h

@@ -82,6 +82,7 @@ public:
     void uninitdataError(const Token *tok, const std::string &varname);
     void uninitvarError(const Token *tok, const std::string &varname);
 
+private:
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckUninitVar c(0, settings, errorLogger);
 

test/testbufferoverrun.cpp

@@ -69,8 +69,6 @@ private:
         TEST_CASE(sizeof2);
         TEST_CASE(sizeof3);
 
-        TEST_CASE(arrayInfo);
-
         TEST_CASE(array_index_1);
         TEST_CASE(array_index_2);
         TEST_CASE(array_index_3);
@@ -345,24 +343,6 @@ private:
 
 
 
-    void arrayInfo() {
-        // Clear the error buffer..
-        errout.str("");
-
-        Settings settings;
-
-        // Tokenize..
-        Tokenizer tokenizer(&settings, this);
-        std::istringstream istr("XY(1) const int a[2] = { 1, 2 };");
-        tokenizer.tokenize(istr, "test.cpp");
-
-        tokenizer.simplifySizeof();
-
-        CheckBufferOverrun::ArrayInfo ai;
-        ASSERT_EQUALS(false, ai.declare(tokenizer.tokens()->tokAt(5), tokenizer));
-    }
-
-
     void array_index_1() {
         check("void f()\n"
               "{\n"
@@ -1032,25 +1012,23 @@ private:
         ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[32768]' index 32768 out of bounds\n"
                       "[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
 
-        if (sizeof(int) == 4) {
+        check("void f(int n) {\n"
-            check("void f(int n) {\n"
+              "    int a[n];\n"     // n <= INT_MAX
-                  "    int a[n];\n"     // n <= INT_MAX
+              "    a[-1] = 0;\n"    // negative index
-                  "    a[-1] = 0;\n"    // negative index
+              "}");
-                  "}\n");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
-            ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
 
-            check("void f(unsigned int n) {\n"
+        check("void f(unsigned int n) {\n"
-                  "    int a[n];\n"     // n <= UINT_MAX
+              "    int a[n];\n"     // n <= UINT_MAX
-                  "    a[-1] = 0;\n"    // negative index
+              "    a[-1] = 0;\n"    // negative index
-                  "}\n");
+              "}");
-            ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
 
-            check("void f(signed int n) {\n"
+        check("void f(signed int n) {\n"
-                  "    int a[n];\n"     // n <= INT_MAX
+              "    int a[n];\n"     // n <= INT_MAX
-                  "    a[-1] = 0;\n"    // negative index
+              "    a[-1] = 0;\n"    // negative index
-                  "}\n");
+              "}");
-            ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
+        ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str());
-        }
     }
 
     void array_index_25() {