From b37cf11d20d06e88c626012e8c123bd772a7c1be Mon Sep 17 00:00:00 2001 From: PKEuS Date: Tue, 27 Mar 2012 19:40:39 +0200 Subject: [PATCH] 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 | 83 ++++---------------------------------- lib/checkbufferoverrun.h | 8 ---- lib/checkstl.h | 25 ++++++------ lib/checkuninitvar.h | 1 + test/testbufferoverrun.cpp | 52 +++++++----------------- 5 files changed, 37 insertions(+), 132 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 6ce5254fd..e20253b39 100644 --- a/lib/checkbufferoverrun.cpp +++ b/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(checks.front()); - std::map::const_iterator it; - it = c->arrayInfo.find(varid1); - if (it == c->arrayInfo.end()) - return; - ai = it->second; - } + ExecutionPathBufferOverrun *c = dynamic_cast(checks.front()); + std::map::const_iterator it1; + it1 = c->arrayInfo.find(varid1); + if (it1 == c->arrayInfo.end()) + return; + const CheckBufferOverrun::ArrayInfo& ai = it1->second; // Check if varid2 variable has a value that is out-of-bounds std::list::const_iterator it; for (it = checks.begin(); it != checks.end(); ++it) { - ExecutionPathBufferOverrun *c = dynamic_cast(*it); + c = dynamic_cast(*it); if (c && c->varId == varid2 && c->value >= ai.num(0)) { // variable value is out of bounds, report error CheckBufferOverrun *checkBufferOverrun = dynamic_cast(c->owner); diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 9d8d089e9..13f511eae 100644 --- a/lib/checkbufferoverrun.h +++ b/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 &num() const { return _num; diff --git a/lib/checkstl.h b/lib/checkstl.h index 3d6167298..17a29080c 100644 --- a/lib/checkstl.h +++ b/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); diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h index 06027735a..d74653121 100644 --- a/lib/checkuninitvar.h +++ b/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); diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 5a2a9cdfa..bd08ef853 100644 --- a/test/testbufferoverrun.cpp +++ b/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" - " int a[n];\n" // n <= INT_MAX - " a[-1] = 0;\n" // negative index - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str()); + check("void f(int n) {\n" + " int a[n];\n" // n <= INT_MAX + " a[-1] = 0;\n" // negative index + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str()); - check("void f(unsigned int n) {\n" - " int a[n];\n" // n <= UINT_MAX - " a[-1] = 0;\n" // negative index - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str()); + check("void f(unsigned int n) {\n" + " int a[n];\n" // n <= UINT_MAX + " a[-1] = 0;\n" // negative index + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str()); - check("void f(signed int n) {\n" - " int a[n];\n" // n <= INT_MAX - " a[-1] = 0;\n" // negative index - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str()); - } + check("void f(signed int n) {\n" + " int a[n];\n" // n <= INT_MAX + " a[-1] = 0;\n" // negative index + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Array index -1 is out of bounds\n", errout.str()); } void array_index_25() {