diff --git a/src/checkautovariables.cpp b/src/checkautovariables.cpp index 85315eaae..fe4e4d97c 100644 --- a/src/checkautovariables.cpp +++ b/src/checkautovariables.cpp @@ -39,164 +39,165 @@ // Register this check class into cppcheck by creating a static instance of it.. namespace { - static CheckAutoVariables instance; +static CheckAutoVariables instance; } // _callStack used when parsing into subfunctions. -bool CheckAutoVariables::error_av(const Token* left,const Token* right) +bool CheckAutoVariables::error_av(const Token* left, const Token* right) { - std::string left_var=left->str(); - std::string right_var=right->str(); - std::list::iterator it_fp; + std::string left_var = left->str(); + std::string right_var = right->str(); + std::list::iterator it_fp; - for(it_fp=fp_list.begin();it_fp!=fp_list.end();it_fp++) - { - std::string vname=(*it_fp); - //cout << "error_av " << vname << " " << left_var << endl; - if (vname==left_var){ - //cout << "Beccato" << endl; - break; //The left argument is a formal parameter - } + for (it_fp = fp_list.begin();it_fp != fp_list.end();it_fp++) + { + std::string vname = (*it_fp); + //cout << "error_av " << vname << " " << left_var << endl; + if (vname == left_var) + { + //cout << "Beccato" << endl; + break; //The left argument is a formal parameter + } - } - if (it_fp==fp_list.end()) - return false; //The left argument is NOT a formal parameter + } + if (it_fp == fp_list.end()) + return false; //The left argument is NOT a formal parameter - std::list::iterator id_vd; - for(id_vd=vd_list.begin();id_vd!=vd_list.end();id_vd++) - { - std::string vname=(*id_vd); - if (vname==right_var) - break; //The left argument is a variable declaration - } - if (id_vd==vd_list.end()) - return false; //The left argument is NOT a variable declaration - //If I reach this point there is a wrong assignement of an auto-variable to an effective parameter of a function - return true; + std::list::iterator id_vd; + for (id_vd = vd_list.begin();id_vd != vd_list.end();id_vd++) + { + std::string vname = (*id_vd); + if (vname == right_var) + break; //The left argument is a variable declaration + } + if (id_vd == vd_list.end()) + return false; //The left argument is NOT a variable declaration + //If I reach this point there is a wrong assignement of an auto-variable to an effective parameter of a function + return true; } bool CheckAutoVariables::is_auto_var(const Token* t) { - std::list::iterator id_vd; - std::string v=t->str(); - for(id_vd=vd_list.begin();id_vd!=vd_list.end();id_vd++) - { - std::string vname=(*id_vd); - if (vname==v) - return true; - } - return false; + std::list::iterator id_vd; + std::string v = t->str(); + for (id_vd = vd_list.begin();id_vd != vd_list.end();id_vd++) + { + std::string vname = (*id_vd); + if (vname == v) + return true; + } + return false; } void print(const Token *tok, int num) { - const Token *t=tok; - std::cout << tok->linenr() << " PRINT "; - for (int i=0;istr() << "] "; - t=t->next(); - } - std::cout << std::endl; + const Token *t = tok; + std::cout << tok->linenr() << " PRINT "; + for (int i = 0;i < num;i++) + { + std::cout << " [" << t->str() << "] "; + t = t->next(); + } + std::cout << std::endl; } bool isTypeName(const Token *tok) { - bool ret = false; - std::string _str=tok->str(); - const char *type[] = {"case", "return","delete",0}; + bool ret = false; + std::string _str = tok->str(); + const char *type[] = {"case", "return", "delete", 0}; for (int i = 0; type[i]; i++) ret |= (_str == type[i]); return !ret; } void CheckAutoVariables::addVD(const Token* tok) { - std::string var_name; - var_name=tok->str(); - //cout << "VD " << tok->linenr() << " " << var_name << endl; - vd_list.push_back(var_name); + std::string var_name; + var_name = tok->str(); + //cout << "VD " << tok->linenr() << " " << var_name << endl; + vd_list.push_back(var_name); } void CheckAutoVariables::autoVariables() { - bool begin_function=false; - bool begin_function_decl=false; - int bindent=0; + bool begin_function = false; + bool begin_function_decl = false; + int bindent = 0; for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::Match(tok, "%type% %var% (") || - Token::Match(tok, "%type% * %var% (") || - Token::Match(tok, "%type% :: %var% (")) - { - begin_function=true; - fp_list.clear(); - vd_list.clear(); - } - else if (begin_function && begin_function_decl && Token::Match(tok, "%type% * %var%")) - { - std::string var_name; + if (Token::Match(tok, "%type% %var% (") || + Token::Match(tok, "%type% * %var% (") || + Token::Match(tok, "%type% :: %var% (")) + { + begin_function = true; + fp_list.clear(); + vd_list.clear(); + } + else if (begin_function && begin_function_decl && Token::Match(tok, "%type% * %var%")) + { + std::string var_name; - var_name=tok->tokAt(2)->str(); - //cout << "FP " << tok->linenr() << " " << var_name << endl; - fp_list.push_back(var_name); - } - else if (begin_function && Token::Match(tok, "(")) - begin_function_decl=true; - else if (begin_function && Token::Match(tok, ")")) - { - begin_function_decl=false; - } - else if (begin_function && Token::Match(tok, "{")) - bindent++; - else if (begin_function && Token::Match(tok, "}")) - { - bindent--; - } - else if (bindent>0 && Token::Match(tok, "%type% :: %any%")) //Inside a function - { - std::string var_name; - //print(tok,5); - var_name=tok->tokAt(2)->str(); - vd_list.push_back(var_name); - } - else if (bindent>0 && Token::Match(tok, "%var% %var% ;")) //Inside a function - { - if (!isTypeName(tok)) - continue; - addVD(tok->tokAt(1)); - } - else if (bindent>0 && Token::Match(tok, "const %var% %var% ;")) //Inside a function - { - if (!isTypeName(tok->tokAt(1))) - continue; - addVD(tok->tokAt(2)); - } - else if (bindent>0 && Token::Match(tok, "%var% = & %var%")) //Critical assignement - { - if (error_av(tok->tokAt(0),tok->tokAt(3))) - _errorLogger->genericError(_tokenizer, - tok, - "Wrong assignement of an auto-variable to an effective parameter of a function"); - } - else if (bindent>0 && Token::Match(tok, "%var% [ %any% ] = & %var%")) //Critical assignement - { - if (error_av(tok->tokAt(0),tok->tokAt(6))) - _errorLogger->genericError(_tokenizer, - tok, - "Wrong assignement of an auto-variable to an effective parameter of a function"); - } - else if (bindent>0 && Token::Match(tok, "return & %var%")) //Critical return - { - if (is_auto_var(tok->tokAt(2))) - _errorLogger->genericError(_tokenizer, - tok, - "Return of the address of an auto-variable"); - } + var_name = tok->tokAt(2)->str(); + //cout << "FP " << tok->linenr() << " " << var_name << endl; + fp_list.push_back(var_name); + } + else if (begin_function && Token::Match(tok, "(")) + begin_function_decl = true; + else if (begin_function && Token::Match(tok, ")")) + { + begin_function_decl = false; + } + else if (begin_function && Token::Match(tok, "{")) + bindent++; + else if (begin_function && Token::Match(tok, "}")) + { + bindent--; + } + else if (bindent > 0 && Token::Match(tok, "%type% :: %any%")) //Inside a function + { + std::string var_name; + //print(tok,5); + var_name = tok->tokAt(2)->str(); + vd_list.push_back(var_name); + } + else if (bindent > 0 && Token::Match(tok, "%var% %var% ;")) //Inside a function + { + if (!isTypeName(tok)) + continue; + addVD(tok->tokAt(1)); + } + else if (bindent > 0 && Token::Match(tok, "const %var% %var% ;")) //Inside a function + { + if (!isTypeName(tok->tokAt(1))) + continue; + addVD(tok->tokAt(2)); + } + else if (bindent > 0 && Token::Match(tok, "%var% = & %var%")) //Critical assignement + { + if (error_av(tok->tokAt(0), tok->tokAt(3))) + _errorLogger->genericError(_tokenizer, + tok, + "Wrong assignement of an auto-variable to an effective parameter of a function"); + } + else if (bindent > 0 && Token::Match(tok, "%var% [ %any% ] = & %var%")) //Critical assignement + { + if (error_av(tok->tokAt(0), tok->tokAt(6))) + _errorLogger->genericError(_tokenizer, + tok, + "Wrong assignement of an auto-variable to an effective parameter of a function"); + } + else if (bindent > 0 && Token::Match(tok, "return & %var%")) //Critical return + { + if (is_auto_var(tok->tokAt(2))) + _errorLogger->genericError(_tokenizer, + tok, + "Return of the address of an auto-variable"); + } } - vd_list.clear(); - fp_list.clear(); + vd_list.clear(); + fp_list.clear(); } //--------------------------------------------------------------------------- diff --git a/src/checkautovariables.h b/src/checkautovariables.h index 5944f4e49..5f187717b 100644 --- a/src/checkautovariables.h +++ b/src/checkautovariables.h @@ -41,11 +41,11 @@ public: /** Check for buffer overruns */ void autoVariables(); private: - std::list fp_list; - std::list vd_list; - bool error_av(const Token* left,const Token* right); - bool is_auto_var(const Token* t); - void addVD(const Token* t); + std::list fp_list; + std::list vd_list; + bool error_av(const Token* left, const Token* right); + bool is_auto_var(const Token* t); + void addVD(const Token* t); const Tokenizer *_tokenizer; const Settings *_settings; ErrorLogger *_errorLogger; diff --git a/test/teststl.cpp b/test/teststl.cpp index c7acdb7f0..39fedcf15 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -49,195 +49,195 @@ private: TEST_CASE(invalidcode); } - void check(const char code[]) - { - // Tokenize.. - Tokenizer tokenizer; - std::istringstream istr(code); - tokenizer.tokenize(istr, "test.cpp"); + void check(const char code[]) + { + // Tokenize.. + Tokenizer tokenizer; + std::istringstream istr(code); + tokenizer.tokenize(istr, "test.cpp"); - // Clear the error buffer.. - errout.str(""); + // Clear the error buffer.. + errout.str(""); - // Check.. - CheckStl checkStl; - checkStl.runChecks(&tokenizer, (const Settings *)0, this); - } + // Check.. + CheckStl checkStl; + checkStl.runChecks(&tokenizer, (const Settings *)0, this); + } - void iterator1() + void iterator1() + { + check("void foo()\n" + "{\n" + " for (it = foo.begin(); it != bar.end(); ++it)\n" + " { }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Same iterator is used with both foo and bar\n", errout.str()); + } + + void iterator2() + { + check("void foo()\n" + "{\n" + " it = foo.begin();\n" + " while (it != bar.end())\n" + " {\n" + " ++it;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Same iterator is used with both foo and bar\n", errout.str()); + } + + + void STLSize() + { + check("void foo()\n" + "{\n" + " std::vector foo;\n" + " for (unsigned int ii = 0; ii <= foo.size(); ++ii)\n" + " {\n" + " foo[ii] = 0;\n" + " }\n" + "}\n"); + ASSERT_EQUALS(std::string("[test.cpp:6]: (error) When ii == size(), foo [ ii ] is out of bounds\n"), errout.str()); + } + + void STLSizeNoErr() + { { check("void foo()\n" "{\n" - " for (it = foo.begin(); it != bar.end(); ++it)\n" - " { }\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Same iterator is used with both foo and bar\n", errout.str()); - } - - void iterator2() - { - check("void foo()\n" - "{\n" - " it = foo.begin();\n" - " while (it != bar.end())\n" + " std::vector foo;\n" + " for (unsigned int ii = 0; ii < foo.size(); ++ii)\n" " {\n" - " ++it;\n" + " foo[ii] = 0;\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Same iterator is used with both foo and bar\n", errout.str()); + ASSERT_EQUALS(std::string(""), errout.str()); } - - void STLSize() { check("void foo()\n" "{\n" " std::vector foo;\n" " for (unsigned int ii = 0; ii <= foo.size(); ++ii)\n" " {\n" - " foo[ii] = 0;\n" " }\n" "}\n"); - ASSERT_EQUALS(std::string("[test.cpp:6]: (error) When ii == size(), foo [ ii ] is out of bounds\n"), errout.str()); + ASSERT_EQUALS(std::string(""), errout.str()); } - void STLSizeNoErr() { - { - check("void foo()\n" - "{\n" - " std::vector foo;\n" - " for (unsigned int ii = 0; ii < foo.size(); ++ii)\n" - " {\n" - " foo[ii] = 0;\n" - " }\n" - "}\n"); - ASSERT_EQUALS(std::string(""), errout.str()); - } - - { - check("void foo()\n" - "{\n" - " std::vector foo;\n" - " for (unsigned int ii = 0; ii <= foo.size(); ++ii)\n" - " {\n" - " }\n" - "}\n"); - ASSERT_EQUALS(std::string(""), errout.str()); - } - - { - check("void foo()\n" - "{\n" - " std::vector foo;\n" - " for (unsigned int ii = 0; ii <= foo.size(); ++ii)\n" - " {\n" - " if (ii == foo.size())\n" - " {\n" - " }\n" - " else\n" - " {\n" - " foo[ii] = 0;\n" - " }\n" - " }\n" - "}\n"); - // TODO ASSERT_EQUALS(std::string(""), errout.str()); - } - } - - - - - - void erase() - { - check("void f()\n" + check("void foo()\n" "{\n" - " for (it = foo.begin(); it != foo.end(); ++it)\n" + " std::vector foo;\n" + " for (unsigned int ii = 0; ii <= foo.size(); ++ii)\n" " {\n" - " foo.erase(it);\n" + " if (ii == foo.size())\n" + " {\n" + " }\n" + " else\n" + " {\n" + " foo[ii] = 0;\n" + " }\n" " }\n" "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (error) Dangerous usage of erase\n", errout.str()); - } - - void eraseBreak() - { - check("void f()\n" - "{\n" - " for (it = foo.begin(); it != foo.end(); ++it)\n" - " {\n" - " foo.erase(it);\n" - " break;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - void eraseReturn() - { - check("void f()\n" - "{\n" - " for (it = foo.begin(); it != foo.end(); ++it)\n" - " {\n" - " foo.erase(it);\n" - " return;\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - void eraseGoto() - { - check("void f()\n" - "{\n" - " for (it = foo.begin(); it != foo.end(); ++it)\n" - " {\n" - " foo.erase(it);\n" - " goto abc;\n" - " }\n" - "bar:\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } - - void eraseAssign() - { - check("void f()\n" - "{\n" - " for (it = foo.begin(); it != foo.end(); ++it)\n" - " {\n" - " foo.erase(it);\n" - " it = foo.begin();\n" - " }\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); + // TODO ASSERT_EQUALS(std::string(""), errout.str()); } + } - void pushback1() - { - check("void f()\n" - "{\n" - " std::vector::const_iterator it = foo.begin();\n" - " foo.push_back(123);\n" - " *it;\n" - "}\n"); - ASSERT_EQUALS("[test.cpp:5]: (error) After push_back or push_front, the iterator 'it' may be invalid\n", errout.str()); - } + void erase() + { + check("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) Dangerous usage of erase\n", errout.str()); + } - void invalidcode() - { - check("void f()\n" - "{\n" - " for ( \n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - } + void eraseBreak() + { + check("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void eraseReturn() + { + check("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " return;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void eraseGoto() + { + check("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " goto abc;\n" + " }\n" + "bar:\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + void eraseAssign() + { + check("void f()\n" + "{\n" + " for (it = foo.begin(); it != foo.end(); ++it)\n" + " {\n" + " foo.erase(it);\n" + " it = foo.begin();\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + + + + + + void pushback1() + { + check("void f()\n" + "{\n" + " std::vector::const_iterator it = foo.begin();\n" + " foo.push_back(123);\n" + " *it;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) After push_back or push_front, the iterator 'it' may be invalid\n", errout.str()); + } + + void invalidcode() + { + check("void f()\n" + "{\n" + " for ( \n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestStl)