From b3e8ef9d483ed98ade583cfb09b5b31193d3056e Mon Sep 17 00:00:00 2001 From: Erik Lax Date: Fri, 11 Feb 2011 06:30:42 +0100 Subject: [PATCH 01/17] Fixed #2559 (Refactoring Preprocessor::read) --- lib/preprocessor.cpp | 150 ++++++++++++++++++++++++++++---------- lib/preprocessor.h | 7 +- test/testpreprocessor.cpp | 76 ++++++++++++++++++- 3 files changed, 192 insertions(+), 41 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index f3ab9e231..36d0d4177 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -109,39 +109,19 @@ static std::string join(const std::list &list, char separator) /** Just read the code into a string. Perform simple cleanup of the code */ std::string Preprocessor::read(std::istream &istr, const std::string &filename, Settings *settings) { - // Get filedata from stream.. - bool ignoreSpace = true; - - // need space.. #if( => #if ( - bool needSpace = false; - + // ------------------------------------------------------------------------------------------ + // // handling // when this is encountered the will be "skipped". // on the next , extra newlines will be added - unsigned int newlines = 0; - std::ostringstream code; + unsigned int newlines = 0; for (unsigned char ch = readChar(istr); istr.good(); ch = readChar(istr)) { // Replace assorted special chars with spaces.. if (((ch & 0x80) == 0) && (ch != '\n') && (std::isspace(ch) || std::iscntrl(ch))) ch = ' '; - // Skip spaces after ' ' and after '#' - if (ch == ' ' && ignoreSpace) - continue; - ignoreSpace = bool(ch == ' ' || ch == '#' || ch == '\n'); - - if (needSpace) - { - if (ch == '(' || ch == '!') - code << " "; - else if (!std::isalpha(ch)) - needSpace = false; - } - if (ch == '#') - needSpace = true; - // .. // for gcc-compatibility the trailing spaces should be ignored // for vs-compatibility the trailing spaces should be kept @@ -178,8 +158,6 @@ std::string Preprocessor::read(std::istream &istr, const std::string &filename, else code << "\\"; } - - // Just some code.. else { code << char(ch); @@ -192,8 +170,117 @@ std::string Preprocessor::read(std::istream &istr, const std::string &filename, } } } + std::string result = code.str(); + code.str(""); - return removeParantheses(removeComments(code.str(), filename, settings)); + // ------------------------------------------------------------------------------------------ + // + // Remove all comments.. + result = removeComments(result, filename, settings); + + // ------------------------------------------------------------------------------------------ + // + // Clean up all preprocessor statements + result = preprocessCleanupDirectives(result); + + // ------------------------------------------------------------------------------------------ + // + // Clean up preprocessor #if statements with Parantheses + result = removeParantheses(result); + + return result; +} + +std::string Preprocessor::preprocessCleanupDirectives(const std::string &processedFile) const +{ + std::ostringstream code; + std::istringstream sstr(processedFile); + + std::string line; + while (std::getline(sstr, line)) + { + // Trim lines.. + if (!line.empty() && line[0] == ' ') + line.erase(0, line.find_first_not_of(" ")); + if (!line.empty() && line[line.size()-1] == ' ') + line.erase(line.find_last_not_of(" ") + 1); + + // Preprocessor + if (!line.empty() && line[0] == '#') + { + enum { + ESC_NONE, + ESC_SINGLE, + ESC_DOUBLE + } escapeStatus = ESC_NONE; + + char prev = ' '; // hack to make it skip spaces between # and the directive + code << "#"; + std::string::const_iterator i = line.begin(); + i++; + + // need space.. #if( => #if ( + bool needSpace = true; + while(i != line.end()) + { + // disable esc-mode + if (escapeStatus != ESC_NONE) + { + if (prev != '\\' && escapeStatus == ESC_SINGLE && *i == '\'') + { + escapeStatus = ESC_NONE; + } + if (prev != '\\' && escapeStatus == ESC_DOUBLE && *i == '"') + { + escapeStatus = ESC_NONE; + } + } else { + // enable esc-mode + if (escapeStatus == ESC_NONE && *i == '"') + escapeStatus = ESC_DOUBLE; + if (escapeStatus == ESC_NONE && *i == '\'') + escapeStatus = ESC_SINGLE; + } + // skip double whitespace between arguments + if (escapeStatus == ESC_NONE && prev == ' ' && *i == ' ') + { + i++; + continue; + } + // Convert #if( to "#if (" + if (escapeStatus == ESC_NONE) + { + if (needSpace) + { + if (*i == '(' || *i == '!') + code << " "; + else if (!std::isalpha(*i)) + needSpace = false; + } + if (*i == '#') + needSpace = true; + } + code << *i; + if (escapeStatus != ESC_NONE && prev == '\\' && *i == '\\') + { + prev = ' '; + } else { + prev = *i; + } + i++; + } + if (escapeStatus != ESC_NONE) + { + // unmatched quotes.. compiler should probably complain about this.. + } + } else { + // Do not mess with regular code.. + code << line; + } + code << (sstr.eof()?"":"\n"); + } + + return code.str(); } static bool hasbom(const std::string &str) @@ -668,9 +755,6 @@ void Preprocessor::preprocess(std::istream &srcCodeStream, std::string &processe processedFile = read(srcCodeStream, filename, _settings); - // normalize the whitespaces of the file - preprocessWhitespaces(processedFile); - // Remove asm(...) removeAsm(processedFile); @@ -1635,15 +1719,7 @@ void Preprocessor::handleIncludes(std::string &code, const std::string &filePath if (!processedFile.empty()) { - // Replace all tabs with spaces.. - std::replace(processedFile.begin(), processedFile.end(), '\t', ' '); - - // Remove all indentation.. - if (!processedFile.empty() && processedFile[0] == ' ') - processedFile.erase(0, processedFile.find_first_not_of(" ")); - // Remove space characters that are after or before new line character - processedFile = removeSpaceNearNL(processedFile); processedFile = "#file \"" + filename + "\"\n" + processedFile + "\n#endfile"; code.insert(pos, processedFile); diff --git a/lib/preprocessor.h b/lib/preprocessor.h index 2d9bc14cb..a6829efea 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -106,7 +106,6 @@ public: * @param processedFile The data to be processed */ static void preprocessWhitespaces(std::string &processedFile); - protected: /** @@ -152,6 +151,12 @@ protected: */ static std::string removeParantheses(const std::string &str); + /** + * clean up #-preprocessor lines (only) + * @param processedFile The data to be processed + */ + std::string preprocessCleanupDirectives(const std::string &processedFile) const; + /** * Returns the string between double quote characters or \< \> characters. * @param str e.g. \code#include "menu.h"\endcode or \code#include \endcode diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index 7163b3af2..fbd012632 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -148,6 +148,7 @@ private: TEST_CASE(macro_simple11); TEST_CASE(macro_simple12); TEST_CASE(macro_simple13); + TEST_CASE(macro_simple14); TEST_CASE(macroInMacro); TEST_CASE(macro_mismatch); TEST_CASE(macro_linenumbers); @@ -198,6 +199,11 @@ private: TEST_CASE(endfile); TEST_CASE(redundant_config); + + TEST_CASE(testPreprocessorRead1); + TEST_CASE(testPreprocessorRead2); + TEST_CASE(testPreprocessorRead3); + TEST_CASE(testPreprocessorRead4); } @@ -209,7 +215,7 @@ private: Preprocessor preprocessor(&settings, this); std::istringstream istr(code); std::string codestr(preprocessor.read(istr,"test.c",0)); - ASSERT_EQUALS("a \n#aa b \n", codestr); + ASSERT_EQUALS("a\n#aa b\n", codestr); } void readCode2() @@ -323,7 +329,7 @@ private: preprocessor.preprocess(istr, actual, "file.c"); // Compare results.. - ASSERT_EQUALS("\n\" #ifdef WIN32\"\n\n\n\n", actual[""]); + ASSERT_EQUALS("\n\" # ifdef WIN32\"\n\n\n\n", actual[""]); ASSERT_EQUALS("\n\n\nqwerty\n\n", actual["WIN32"]); ASSERT_EQUALS(2, static_cast(actual.size())); } @@ -1363,7 +1369,7 @@ private: std::istringstream istr(filedata); Settings settings; Preprocessor preprocessor(&settings, this); - ASSERT_EQUALS("#define str \"abc\" \"def\" \n\nabcdef = str;\n", preprocessor.read(istr, "test.c", 0)); + ASSERT_EQUALS("#define str \"abc\" \"def\"\n\nabcdef = str;\n", preprocessor.read(istr, "test.c", 0)); } void multiline2() @@ -1576,6 +1582,13 @@ private: ASSERT_EQUALS("\n\n", OurPreprocessor::expandMacros(filedata)); } + void macro_simple14() + { + const char filedata[] = "#define A \" a \"\n" + "printf(A);\n"; + ASSERT_EQUALS("\nprintf(\" a \");\n", OurPreprocessor::expandMacros(filedata)); + } + void macroInMacro() { { @@ -2591,6 +2604,63 @@ private: ASSERT_EQUALS("A;A;B is NOT checked", "A;A;B is NOT checked"); } } + + void testPreprocessorRead1() + { + const std::string filedata("/*\n*/ # /*\n*/ defi\\\nne FO\\\nO 10\\\n20"); + std::istringstream istr(filedata.c_str()); + Settings settings; + Preprocessor preprocessor(&settings, this); + ASSERT_EQUALS("#define FOO 1020", preprocessor.read(istr, "test.cpp", 0)); + } + + void testPreprocessorRead2() + { + const std::string filedata("\"foo\\\\\nbar\""); + std::istringstream istr(filedata.c_str()); + Settings settings; + Preprocessor preprocessor(&settings, this); + ASSERT_EQUALS("\"foo\\bar\"", preprocessor.read(istr, "test.cpp", 0)); + } + + void testPreprocessorRead3() + { + const std::string filedata("#define A \" a \"\n\" b\""); + std::istringstream istr(filedata.c_str()); + Settings settings; + Preprocessor preprocessor(&settings, this); + ASSERT_EQUALS(filedata, preprocessor.read(istr, "test.cpp", 0)); + } + + void testPreprocessorRead4() + { + { + // test < \\> < > (unescaped) + const std::string filedata("#define A \" \\\\\"/*space*/ \" \""); + std::istringstream istr(filedata.c_str()); + Settings settings; + Preprocessor preprocessor(&settings, this); + ASSERT_EQUALS("#define A \" \\\\\" \" \"", preprocessor.read(istr, "test.cpp", 0)); + } + + { + // test <" \\\" "> (unescaped) + const std::string filedata("#define A \" \\\\\\\" \""); + std::istringstream istr(filedata.c_str()); + Settings settings; + Preprocessor preprocessor(&settings, this); + ASSERT_EQUALS("#define A \" \\\\\\\" \"", preprocessor.read(istr, "test.cpp", 0)); + } + + { + // test <" \\\\"> <" "> (unescaped) + const std::string filedata("#define A \" \\\\\\\\\"/*space*/ \" \""); + std::istringstream istr(filedata.c_str()); + Settings settings; + Preprocessor preprocessor(&settings, this); + ASSERT_EQUALS("#define A \" \\\\\\\\\" \" \"", preprocessor.read(istr, "test.cpp", 0)); + } + } }; REGISTER_TEST(TestPreprocessor) From b8c5426bb88d0f21e1cb4e9c112c63ec36405fc7 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Fri, 11 Feb 2011 08:00:41 -0500 Subject: [PATCH 02/17] fix #2567 Unused private function when implemented in different file --- lib/checkclass.cpp | 99 ++++++++----------------------------- test/testunusedprivfunc.cpp | 22 +++++++++ 2 files changed, 42 insertions(+), 79 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 53dd353a2..0d71e4ad0 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -610,7 +610,9 @@ void CheckClass::privateFunctions() if (!func->hasBody) { // empty private copy constructors and assignment operators are OK - if ((func->type == Function::eCopyConstructor || func->type == Function::eOperatorEqual) && func->access == Private) + if ((func->type == Function::eCopyConstructor || + func->type == Function::eOperatorEqual) && + func->access == Private) continue; whole = false; @@ -630,98 +632,37 @@ void CheckClass::privateFunctions() for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { // Get private functions.. - if (func->type == Function::eFunction && - func->access == Private && func->hasBody) + if (func->type == Function::eFunction && func->access == Private) FuncList.push_back(func->tokenDef); } } // Check that all private functions are used.. - bool HasFuncImpl = false; - bool inclass = false; - int indent_level = 0; - for (const Token *ftok = _tokenizer->tokens(); ftok; ftok = ftok->next()) + for (func = scope->functionList.begin(); func != scope->functionList.end(); ++func) { - if (ftok->str() == "{") - ++indent_level; - else if (ftok->str() == "}") + const Token *ftok = func->arg->link()->next(); + while (ftok->str() != "{") + ftok = ftok->next(); + const Token *etok = ftok->link(); + + for (; ftok != etok; ftok = ftok->next()) { - if (indent_level > 0) - --indent_level; - if (indent_level == 0) - inclass = false; - } - - else if (ftok->str() == "class" && - ftok->next()->str() == classname && - Token::Match(ftok->tokAt(2), ":|{")) - { - indent_level = 0; - inclass = true; - } - - // Check member class functions to see what functions are used.. - else if ((inclass && indent_level == 1 && Token::Match(ftok, "%var% (")) || - (ftok->str() == classname && Token::Match(ftok->next(), ":: ~| %var% ("))) - { - while (ftok && ftok->str() != ")") - ftok = ftok->next(); - if (!ftok) - break; - if (Token::Match(ftok, ") : %var% (")) + if (Token::Match(ftok, "%var% (")) { - while (!Token::Match(ftok->next(), "[{};]")) + // Remove function from FuncList + std::list::iterator it = FuncList.begin(); + while (it != FuncList.end()) { - if (Token::Match(ftok, "::|,|( %var% ,|)")) - { - // Remove function from FuncList - std::list::iterator it = FuncList.begin(); - while (it != FuncList.end()) - { - if (ftok->next()->str() == (*it)->str()) - FuncList.erase(it++); - else - ++it; - } - } - ftok = ftok->next(); - } - } - if (!Token::Match(ftok, ") const| {")) - continue; - - if (ftok->fileIndex() == 0) - HasFuncImpl = true; - - // Parse function.. - int indentlevel2 = 0; - for (const Token *tok2 = ftok; tok2; tok2 = tok2->next()) - { - if (tok2->str() == "{") - ++indentlevel2; - else if (tok2->str() == "}") - { - --indentlevel2; - if (indentlevel2 < 1) - break; - } - else if (Token::Match(tok2, "%var% (")) - { - // Remove function from FuncList - std::list::iterator it = FuncList.begin(); - while (it != FuncList.end()) - { - if (tok2->str() == (*it)->str()) - FuncList.erase(it++); - else - ++it; - } + if (ftok->str() == (*it)->str()) + FuncList.erase(it++); + else + ++it; } } } } - while (HasFuncImpl && !FuncList.empty()) + while (!FuncList.empty()) { // Final check; check if the function pointer is used somewhere.. const std::string _pattern("return|(|)|,|= " + FuncList.front()->str()); diff --git a/test/testunusedprivfunc.cpp b/test/testunusedprivfunc.cpp index adbbf2b20..c9358aae8 100644 --- a/test/testunusedprivfunc.cpp +++ b/test/testunusedprivfunc.cpp @@ -65,6 +65,8 @@ private: TEST_CASE(testDoesNotIdentifyMethodAsFirstFunctionArgument); // #2480 TEST_CASE(testDoesNotIdentifyMethodAsMiddleFunctionArgument); TEST_CASE(testDoesNotIdentifyMethodAsLastFunctionArgument); + + TEST_CASE(multiFile); } @@ -549,6 +551,26 @@ private: ); ASSERT_EQUALS("", errout.str()); } + + void multiFile() // ticket #2567 + { + check("#file \"test.h\"\n" + "struct Fred\n" + "{\n" + " Fred()\n" + " {\n" + " Init();\n" + " }\n" + "private:\n" + " void Init();\n" + "};\n" + "#endfile\n" + "void Fred::Init()\n" + "{\n" + "}\n"); + + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestUnusedPrivateFunction) From 751f8d46e5e7deafbdcdc48adf02f212b9e27304 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Fri, 11 Feb 2011 18:01:27 +0100 Subject: [PATCH 03/17] Fixed #2570 (Preprocessor: #define parsing when there is no whitespace between a macro symbol and its double-quoted string expansion) --- lib/preprocessor.cpp | 6 ++++-- test/testpreprocessor.cpp | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 36d0d4177..557fea4fa 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -2124,12 +2124,14 @@ public: else if (_params.empty()) { - std::string::size_type pos = _macro.find(" "); + std::string::size_type pos = _macro.find_first_of(" \""); if (pos == std::string::npos) macrocode = ""; else { - macrocode = _macro.substr(pos + 1); + if (_macro[pos] == ' ') + pos++; + macrocode = _macro.substr(pos); if ((pos = macrocode.find_first_of("\r\n")) != std::string::npos) macrocode.erase(pos); } diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index fbd012632..fdeb61d22 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -149,6 +149,7 @@ private: TEST_CASE(macro_simple12); TEST_CASE(macro_simple13); TEST_CASE(macro_simple14); + TEST_CASE(macro_simple15); TEST_CASE(macroInMacro); TEST_CASE(macro_mismatch); TEST_CASE(macro_linenumbers); @@ -1589,6 +1590,13 @@ private: ASSERT_EQUALS("\nprintf(\" a \");\n", OurPreprocessor::expandMacros(filedata)); } + void macro_simple15() + { + const char filedata[] = "#define FOO\"foo\"\n" + "FOO\n"; + ASSERT_EQUALS("\n\"foo\"\n", OurPreprocessor::expandMacros(filedata)); + } + void macroInMacro() { { From f2f2d1f885d034b26e98fe0d6a9e5a0d97c86bb3 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Fri, 11 Feb 2011 18:51:22 +0100 Subject: [PATCH 04/17] Fixed #2571 (Preprocessor: better handling for #undef) --- lib/preprocessor.cpp | 3 ++- test/testpreprocessor.cpp | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 557fea4fa..638e07b29 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -1544,7 +1544,8 @@ std::string Preprocessor::getcode(const std::string &filedata, std::string cfg, return ""; } - if (!match && line.compare(0, 8, "#define ") == 0) + if (!match && (line.compare(0, 8, "#define ") == 0 || + line.compare(0, 6, "#undef") == 0)) { // Remove define that is not part of this configuration line = ""; diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index fdeb61d22..09795e370 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -2483,6 +2483,25 @@ private: ASSERT_EQUALS("\n\n1\n\n", actual[""]); ASSERT_EQUALS(1, (int)actual.size()); } + + { + const char filedata[] = "#define A 1\n" + "#if 0\n" + "#undef A\n" + "#endif\n" + "A\n"; + + // Preprocess => actual result.. + std::istringstream istr(filedata); + std::map actual; + Settings settings; + Preprocessor preprocessor(&settings, this); + preprocessor.preprocess(istr, actual, "file.c"); + + // Compare results.. + ASSERT_EQUALS("\n\n\n\n1\n", actual[""]); + ASSERT_EQUALS(1, (int)actual.size()); + } } void define_ifndef1() From c7821675dd712e864bba9d54ef6d46dbde0a274b Mon Sep 17 00:00:00 2001 From: Erik Lax Date: Fri, 11 Feb 2011 18:57:58 +0100 Subject: [PATCH 05/17] Preprocessor: Test handling of strings with multiple spaces (Ticket: #2548) --- test/testbufferoverrun.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 4966231ba..189e4364b 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -133,6 +133,7 @@ private: TEST_CASE(buffer_overrun_14); TEST_CASE(buffer_overrun_15); // ticket #1787 TEST_CASE(buffer_overrun_16); + TEST_CASE(buffer_overrun_17); // ticket #2548 TEST_CASE(buffer_overrun_bailoutIfSwitch); // ticket #2378 : bailoutIfSwitch // It is undefined behaviour to point out of bounds of an array @@ -1860,6 +1861,15 @@ private: ASSERT_EQUALS("", errout.str()); } + void buffer_overrun_17() // ticket #2548 + { + check("void f() {\n" + " char t[8];\n" + " sprintf(t, \"%s\", \"foo bar\");\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Buffer access out-of-bounds\n", errout.str()); + } + void buffer_overrun_bailoutIfSwitch() { // No false positive From 227a6100f7a5e8cda4fa18aceb720b88a6bb82ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 11 Feb 2011 19:31:37 +0100 Subject: [PATCH 06/17] astyle formatting --- lib/checkclass.cpp | 2 +- lib/preprocessor.cpp | 17 ++++++++++++----- lib/preprocessor.h | 2 +- tools/dmake.cpp | 2 +- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 0d71e4ad0..ddbef4288 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -644,7 +644,7 @@ void CheckClass::privateFunctions() while (ftok->str() != "{") ftok = ftok->next(); const Token *etok = ftok->link(); - + for (; ftok != etok; ftok = ftok->next()) { if (Token::Match(ftok, "%var% (")) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 638e07b29..091da7ea0 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -208,7 +208,8 @@ std::string Preprocessor::preprocessCleanupDirectives(const std::string &process // Preprocessor if (!line.empty() && line[0] == '#') { - enum { + enum + { ESC_NONE, ESC_SINGLE, ESC_DOUBLE @@ -221,7 +222,7 @@ std::string Preprocessor::preprocessCleanupDirectives(const std::string &process // need space.. #if( => #if ( bool needSpace = true; - while(i != line.end()) + while (i != line.end()) { // disable esc-mode if (escapeStatus != ESC_NONE) @@ -234,7 +235,9 @@ std::string Preprocessor::preprocessCleanupDirectives(const std::string &process { escapeStatus = ESC_NONE; } - } else { + } + else + { // enable esc-mode if (escapeStatus == ESC_NONE && *i == '"') escapeStatus = ESC_DOUBLE; @@ -264,7 +267,9 @@ std::string Preprocessor::preprocessCleanupDirectives(const std::string &process if (escapeStatus != ESC_NONE && prev == '\\' && *i == '\\') { prev = ' '; - } else { + } + else + { prev = *i; } i++; @@ -273,7 +278,9 @@ std::string Preprocessor::preprocessCleanupDirectives(const std::string &process { // unmatched quotes.. compiler should probably complain about this.. } - } else { + } + else + { // Do not mess with regular code.. code << line; } diff --git a/lib/preprocessor.h b/lib/preprocessor.h index a6829efea..82f5da2bd 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -152,7 +152,7 @@ protected: static std::string removeParantheses(const std::string &str); /** - * clean up #-preprocessor lines (only) + * clean up #-preprocessor lines (only) * @param processedFile The data to be processed */ std::string preprocessCleanupDirectives(const std::string &processedFile) const; diff --git a/tools/dmake.cpp b/tools/dmake.cpp index 08da9124c..ea7cdcb34 100644 --- a/tools/dmake.cpp +++ b/tools/dmake.cpp @@ -251,7 +251,7 @@ int main(int argc, char **argv) makeConditionalVariable(fout, "INCLUDE_FOR_LIB", "-Ilib"); makeConditionalVariable(fout, "INCLUDE_FOR_CLI", "-Ilib -Iexternals -Iexternals/tinyxml"); makeConditionalVariable(fout, "INCLUDE_FOR_TEST", "-Ilib -Icli -Iexternals -Iexternals/tinyxml"); - + fout << "BIN=$(DESTDIR)$(PREFIX)/bin\n\n"; fout << "# For 'make man': sudo apt-get install xsltproc docbook-xsl docbook-xml on Linux\n"; fout << "DB2MAN=/usr/share/sgml/docbook/stylesheet/xsl/nwalsh/manpages/docbook.xsl\n"; From a1c123459611cf8222aba2de4eec3ab77631498a Mon Sep 17 00:00:00 2001 From: Erik Lax Date: Fri, 11 Feb 2011 21:03:39 +0200 Subject: [PATCH 07/17] Fixed: #2460 (GUI: Errors are sorted by line number alphabetically instead of numerically) --- gui/resultstree.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gui/resultstree.cpp b/gui/resultstree.cpp index a54f1757f..82ebd62fd 100644 --- a/gui/resultstree.cpp +++ b/gui/resultstree.cpp @@ -87,8 +87,9 @@ QStandardItem *ResultsTree::CreateNormalItem(const QString &name) QStandardItem *ResultsTree::CreateLineNumberItem(const QString &linenumber) { - QStandardItem *item = new QStandardItem(linenumber); - item->setData(linenumber, Qt::ToolTipRole); + QStandardItem *item = new QStandardItem(); + item->setData(QVariant(linenumber.toULongLong()), Qt::DisplayRole); + item->setToolTip(linenumber); item->setTextAlignment(Qt::AlignRight | Qt::AlignVCenter); item->setEditable(false); return item; From eddbfbee1eb7480e171179e58a1f3f1b9eadb936 Mon Sep 17 00:00:00 2001 From: Erik Lax Date: Fri, 11 Feb 2011 21:08:37 +0200 Subject: [PATCH 08/17] Fixed: #2572 (GUI: Disable all UI actions on Recheck) --- gui/mainwindow.cpp | 20 +++++++++++++------- gui/mainwindow.h | 6 ++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index bd32a1334..c7b966b6e 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -213,14 +213,11 @@ void MainWindow::DoCheckFiles(const QStringList &files) QDir inf(mCurrentDirectory); const QString checkPath = inf.canonicalPath(); mSettings->setValue(SETTINGS_CHECK_PATH, checkPath); - EnableCheckButtons(false); - mUI.mActionSettings->setEnabled(false); - mUI.mActionOpenXML->setEnabled(false); - mUI.mResults->SetCheckDirectory(checkPath); + CheckLockDownUI(); // lock UI while checking + + mUI.mResults->SetCheckDirectory(checkPath); Settings checkSettings = GetCppcheckSettings(); - EnableProjectActions(false); - EnableProjectOpenActions(false); mThread->Check(checkSettings, false); } @@ -400,6 +397,15 @@ void MainWindow::CheckDone() QApplication::alert(this, 3000); } +void MainWindow::CheckLockDownUI() +{ + EnableCheckButtons(false); + mUI.mActionSettings->setEnabled(false); + mUI.mActionOpenXML->setEnabled(false); + EnableProjectActions(false); + EnableProjectOpenActions(false); +} + void MainWindow::ProgramSettings() { SettingsDialog dialog(mSettings, mApplications, mTranslation, this); @@ -418,7 +424,7 @@ void MainWindow::ProgramSettings() void MainWindow::ReCheck() { ClearResults(); - EnableCheckButtons(false); + CheckLockDownUI(); // lock UI while checking const int filesCount = mThread->GetPreviousFilesCount(); Q_ASSERT(filesCount > 0); // If no files should not be able to recheck diff --git a/gui/mainwindow.h b/gui/mainwindow.h index ea844a287..9e3361c05 100644 --- a/gui/mainwindow.h +++ b/gui/mainwindow.h @@ -206,6 +206,12 @@ protected slots: */ void CheckDone(); + /** + * @brief Lock down UI while checking + * + */ + void CheckLockDownUI(); + /** * @brief Slot for enabling save and clear button * From 951a81d0d201aceec929a5a47e85b1ec7e80df50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 11 Feb 2011 20:12:51 +0100 Subject: [PATCH 09/17] Tokenizer::simplifyKnownVariables: Broke out the simplification into a separate function --- lib/tokenize.cpp | 794 ++++++++++++++++++++++++----------------------- lib/tokenize.h | 6 + 2 files changed, 407 insertions(+), 393 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 3ec631385..00c61dcad 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -6297,399 +6297,7 @@ bool Tokenizer::simplifyKnownVariables() tok3 = tok2->next(); } - Token* bailOutFromLoop = 0; - int indentlevel3 = indentlevel; // indentlevel for tok3 - bool ret3 = false; - for (; tok3; tok3 = tok3->next()) - { - if (tok3->str() == "{") - { - ++indentlevel3; - } - else if (tok3->str() == "}") - { - --indentlevel3; - if (indentlevel3 < indentlevel) - { - if (Token::Match(tok2->tokAt(-7), "%type% * %var% ; %var% = & %var% ;") && - tok2->tokAt(-5)->str() == tok2->tokAt(-3)->str()) - { - tok2 = tok2->tokAt(-4); - Token::eraseTokens(tok2, tok2->tokAt(5)); - } - break; - } - } - - // Stop if label is found - if (Token::Match(tok3, "; %type% : ;")) - break; - - // Stop if return or break is found .. - if (tok3->str() == "break") - break; - if ((indentlevel3 > 1 || !Token::simpleMatch(Token::findmatch(tok3,";"), "; }")) && tok3->str() == "return") - ret3 = true; - if (ret3 && tok3->str() == ";") - break; - - if (pointeralias && Token::Match(tok3, ("!!= " + value).c_str())) - break; - - // Stop if do is found - if (tok3->str() == "do") - break; - - // Stop if something like 'while (--var)' is found - if (tok3->str() == "for" || tok3->str() == "while" || tok3->str() == "do") - { - const Token *endpar = tok3->next()->link(); - if (Token::simpleMatch(endpar, ") {")) - endpar = endpar->next()->link(); - bool bailout = false; - for (const Token *tok4 = tok3; tok4 && tok4 != endpar; tok4 = tok4->next()) - { - if (Token::Match(tok4, "++|-- %varid%", varid) || - Token::Match(tok4, "%varid% ++|--|=", varid)) - { - bailout = true; - break; - } - } - if (bailout) - break; - } - - if (bailOutFromLoop) - { - // This could be a loop, skip it, but only if it doesn't contain - // the variable we are checking for. If it contains the variable - // we will bail out. - if (tok3->varId() == varid) - { - // Continue - //tok2 = bailOutFromLoop; - break; - } - else if (tok3 == bailOutFromLoop) - { - // We have skipped the loop - bailOutFromLoop = 0; - continue; - } - - continue; - } - else if (tok3->str() == "{" && tok3->previous()->str() == ")") - { - // There is a possible loop after the assignment. Try to skip it. - if (tok3->previous()->link() && - !Token::simpleMatch(tok3->previous()->link()->previous(), "if")) - bailOutFromLoop = tok3->link(); - continue; - } - - // Variable used in realloc (see Ticket #1649) - if (Token::Match(tok3, "%var% = realloc ( %var% ,") && - tok3->varId() == varid && - tok3->tokAt(4)->varId() == varid) - { - tok3->tokAt(4)->str(value); - ret = true; - } - - // condition "(|&&|%OROR% %varid% )|&&|%OROR% - if (!Token::Match(tok3->previous(), "( %var% )") && - (Token::Match(tok3->previous(), "&&|(") || tok3->strAt(-1) == "||") && - tok3->varId() == varid && - (Token::Match(tok3->next(), "&&|)") || tok3->strAt(1) == "||")) - { - tok3->str(value); - ret = true; - } - - // Variable is used somehow in a non-defined pattern => bail out - if (tok3->varId() == varid) - { - // This is a really generic bailout so let's try to avoid this. - // There might be lots of false negatives. - if (_settings->debugwarnings) - { - // FIXME: Fix all the debug warnings for values and then - // remove this bailout - if (pointeralias) - break; - - // suppress debug-warning when calling member function - if (Token::Match(tok3->next(), ". %var% (")) - break; - - // suppress debug-warning when assignment - if (Token::simpleMatch(tok3->next(), "=")) - break; - - // taking address of variable.. - if (Token::Match(tok3->tokAt(-2), "return|= & %var% ;")) - break; - - // parameter in function call.. - if (Token::Match(tok3->tokAt(-2), "%var% ( %var% ,|)") || - Token::Match(tok3->previous(), ", %var% ,|)")) - break; - - // conditional increment - if (Token::Match(tok3->tokAt(-3), ") { ++|--") || - Token::Match(tok3->tokAt(-2), ") { %var% ++|--")) - break; - - std::list locationList; - ErrorLogger::ErrorMessage::FileLocation loc; - loc.line = tok3->linenr(); - loc.setfile(file(tok3)); - locationList.push_back(loc); - - const ErrorLogger::ErrorMessage errmsg(locationList, - Severity::debug, - "simplifyKnownVariables: bailing out (variable="+tok3->str()+", value="+value+")", - "debug"); - - if (_errorLogger) - _errorLogger->reportErr(errmsg); - else - Check::reportError(errmsg); - } - - break; - } - - // Using the variable in condition.. - if (Token::Match(tok3->previous(), ("if ( " + structname + " %varid% ==|!=|<|<=|>|>=|)").c_str(), varid) || - Token::Match(tok3, ("( " + structname + " %varid% ==|!=|<|<=|>|>=").c_str(), varid) || - Token::Match(tok3, ("!|==|!=|<|<=|>|>= " + structname + " %varid% ==|!=|<|<=|>|>=|)").c_str(), varid) || - Token::Match(tok3->previous(), "strlen|free ( %varid% )", varid)) - { - if (!structname.empty()) - { - tok3->deleteNext(); - tok3->deleteNext(); - } - tok3 = tok3->next(); - tok3->str(value); - tok3->varId(valueVarId); - ret = true; - } - - // Delete pointer alias - if (pointeralias && tok3->str() == "delete" && - (Token::Match(tok3, "delete %varid% ;", varid) || - Token::Match(tok3, "delete [ ] %varid%", varid))) - { - tok3 = (tok3->strAt(1) == "[") ? tok3->tokAt(3) : tok3->next(); - tok3->str(value); - tok3->varId(valueVarId); - ret = true; - } - - // Variable is used in function call.. - if (Token::Match(tok3, ("%var% ( " + structname + " %varid% ,").c_str(), varid)) - { - const char * const functionName[] = - { - "memcmp","memcpy","memmove","memset", - "strcmp","strcpy","strncpy","strdup" - }; - for (unsigned int i = 0; i < (sizeof(functionName) / sizeof(*functionName)); ++i) - { - if (tok3->str() == functionName[i]) - { - Token *par1 = tok3->next()->next(); - if (!structname.empty()) - { - par1->deleteThis(); - par1->deleteThis(); - } - par1->str(value); - par1->varId(valueVarId); - break; - } - } - } - - // Variable is used as 2nd parameter in function call.. - if (Token::Match(tok3, ("%var% ( %any% , " + structname + " %varid% ,|)").c_str(), varid)) - { - const char * const functionName[] = - { - "memcmp","memcpy","memmove", - "strcmp","strcpy","strncmp","strncpy" - }; - for (unsigned int i = 0; i < (sizeof(functionName) / sizeof(*functionName)); ++i) - { - if (tok3->str() == functionName[i]) - { - Token *par = tok3->tokAt(4); - if (!structname.empty()) - { - par->deleteThis(); - par->deleteThis(); - } - par->str(value); - par->varId(valueVarId); - break; - } - } - } - - // array usage - if (Token::Match(tok3, ("[(,] " + structname + " %varid% [+-*/[]").c_str(), varid)) - { - if (!structname.empty()) - { - tok3->deleteNext(); - tok3->deleteNext(); - } - tok3 = tok3->next(); - tok3->str(value); - tok3->varId(valueVarId); - ret = true; - } - - // Variable is used in calculation.. - if (((tok3->previous()->varId() > 0) && Token::Match(tok3, ("& " + structname + " %varid%").c_str(), varid)) || - Token::Match(tok3, ("[=+-*/[] " + structname + " %varid% [=?+-*/;])]").c_str(), varid) || - Token::Match(tok3, ("[(=+-*/[] " + structname + " %varid% <<|>>").c_str(), varid) || - Token::Match(tok3, ("<<|>> " + structname + " %varid% [+-*/;])]").c_str(), varid) || - Token::Match(tok3->previous(), ("[=+-*/[] ( " + structname + " %varid%").c_str(), varid)) - { - if (!structname.empty()) - { - tok3->deleteNext(); - tok3->deleteNext(); - } - tok3 = tok3->next(); - tok3->str(value); - tok3->varId(valueVarId); - if (tok3->previous()->str() == "*" && valueIsPointer) - { - tok3 = tok3->previous(); - tok3->deleteThis(); - } - ret = true; - } - - if (Token::simpleMatch(tok3, "= {")) - { - unsigned int indentlevel4 = 0; - for (const Token *tok4 = tok3; tok4; tok4 = tok4->next()) - { - if (tok4->str() == "{") - ++indentlevel4; - else if (tok4->str() == "}") - { - if (indentlevel4 <= 1) - break; - --indentlevel4; - } - if (Token::Match(tok4, "{|, %varid% ,|}", varid)) - { - tok4->next()->str(value); - tok4->next()->varId(valueVarId); - ret = true; - } - } - } - - // Using the variable in for-condition.. - if (Token::simpleMatch(tok3, "for (")) - { - for (Token *tok4 = tok3->tokAt(2); tok4; tok4 = tok4->next()) - { - if (tok4->str() == "(" || tok4->str() == ")") - break; - - // Replace variable used in condition.. - if (Token::Match(tok4, "; %var% <|<=|!= %var% ; ++| %var% ++| )")) - { - const Token *inctok = tok4->tokAt(5); - if (inctok->str() == "++") - inctok = inctok->next(); - if (inctok->varId() == varid) - break; - - if (tok4->next()->varId() == varid) - { - tok4->next()->str(value); - tok4->next()->varId(valueVarId); - ret = true; - } - if (tok4->tokAt(3)->varId() == varid) - { - tok4->tokAt(3)->str(value); - tok4->tokAt(3)->varId(valueVarId); - ret = true; - } - } - } - } - - if (indentlevel == indentlevel3 && Token::Match(tok3->next(), "%varid% ++|--", varid) && MathLib::isInt(value)) - { - const std::string op(tok3->strAt(2)); - if (Token::Match(tok3, "[{};] %any% %any% ;")) - { - Token::eraseTokens(tok3, tok3->tokAt(3)); - } - else - { - tok3 = tok3->next(); - tok3->str(value); - tok3->varId(valueVarId); - tok3->deleteNext(); - } - incdec(value, op); - if (!Token::simpleMatch(tok2->tokAt(-2), "for (")) - { - tok2->tokAt(2)->str(value); - tok2->tokAt(2)->varId(valueVarId); - } - ret = true; - } - - if (indentlevel == indentlevel3 && Token::Match(tok3->next(), "++|-- %varid%", varid) && MathLib::isInt(value) && - !Token::Match(tok3->tokAt(3), "[.[]")) - { - incdec(value, tok3->strAt(1)); - tok2->tokAt(2)->str(value); - tok2->tokAt(2)->varId(valueVarId); - if (Token::Match(tok3, "[;{}] %any% %any% ;")) - { - Token::eraseTokens(tok3, tok3->tokAt(3)); - } - else - { - tok3->deleteNext(); - tok3->next()->str(value); - tok3->next()->varId(valueVarId); - } - tok3 = tok3->next(); - ret = true; - } - - // return variable.. - if (Token::Match(tok3, "return %varid% %any%", varid) && - isOp(tok3->tokAt(2))) - { - tok3->next()->str(value); - tok3->next()->varId(valueVarId); - } - - else if (pointeralias && Token::Match(tok3, "return * %varid% ;", varid)) - { - tok3->deleteNext(); - tok3->next()->str(value); - tok3->next()->varId(valueVarId); - } - } + ret |= simplifyKnownVariablesSimplify(&tok2, tok3, varid, structname, value, valueVarId, valueIsPointer, pointeralias, indentlevel); } } @@ -6700,6 +6308,406 @@ bool Tokenizer::simplifyKnownVariables() return ret; } +bool Tokenizer::simplifyKnownVariablesSimplify(Token **tok2, Token *tok3, unsigned int varid, const std::string &structname, std::string &value, unsigned int valueVarId, bool valueIsPointer, bool pointeralias, int indentlevel) +{ + bool ret = false;; + + Token* bailOutFromLoop = 0; + int indentlevel3 = indentlevel; + bool ret3 = false; + for (; tok3; tok3 = tok3->next()) + { + if (tok3->str() == "{") + { + ++indentlevel3; + } + else if (tok3->str() == "}") + { + --indentlevel3; + if (indentlevel3 < indentlevel) + { + if (Token::Match((*tok2)->tokAt(-7), "%type% * %var% ; %var% = & %var% ;") && + (*tok2)->tokAt(-5)->str() == (*tok2)->tokAt(-3)->str()) + { + (*tok2) = (*tok2)->tokAt(-4); + Token::eraseTokens((*tok2), (*tok2)->tokAt(5)); + } + break; + } + } + + // Stop if label is found + if (Token::Match(tok3, "; %type% : ;")) + break; + + // Stop if return or break is found .. + if (tok3->str() == "break") + break; + if ((indentlevel3 > 1 || !Token::simpleMatch(Token::findmatch(tok3,";"), "; }")) && tok3->str() == "return") + ret3 = true; + if (ret3 && tok3->str() == ";") + break; + + if (pointeralias && Token::Match(tok3, ("!!= " + value).c_str())) + break; + + // Stop if do is found + if (tok3->str() == "do") + break; + + // Stop if something like 'while (--var)' is found + if (tok3->str() == "for" || tok3->str() == "while" || tok3->str() == "do") + { + const Token *endpar = tok3->next()->link(); + if (Token::simpleMatch(endpar, ") {")) + endpar = endpar->next()->link(); + bool bailout = false; + for (const Token *tok4 = tok3; tok4 && tok4 != endpar; tok4 = tok4->next()) + { + if (Token::Match(tok4, "++|-- %varid%", varid) || + Token::Match(tok4, "%varid% ++|--|=", varid)) + { + bailout = true; + break; + } + } + if (bailout) + break; + } + + if (bailOutFromLoop) + { + // This could be a loop, skip it, but only if it doesn't contain + // the variable we are checking for. If it contains the variable + // we will bail out. + if (tok3->varId() == varid) + { + // Continue + //tok2 = bailOutFromLoop; + break; + } + else if (tok3 == bailOutFromLoop) + { + // We have skipped the loop + bailOutFromLoop = 0; + continue; + } + + continue; + } + else if (tok3->str() == "{" && tok3->previous()->str() == ")") + { + // There is a possible loop after the assignment. Try to skip it. + if (tok3->previous()->link() && + !Token::simpleMatch(tok3->previous()->link()->previous(), "if")) + bailOutFromLoop = tok3->link(); + continue; + } + + // Variable used in realloc (see Ticket #1649) + if (Token::Match(tok3, "%var% = realloc ( %var% ,") && + tok3->varId() == varid && + tok3->tokAt(4)->varId() == varid) + { + tok3->tokAt(4)->str(value); + ret = true; + } + + // condition "(|&&|%OROR% %varid% )|&&|%OROR% + if (!Token::Match(tok3->previous(), "( %var% )") && + (Token::Match(tok3->previous(), "&&|(") || tok3->strAt(-1) == "||") && + tok3->varId() == varid && + (Token::Match(tok3->next(), "&&|)") || tok3->strAt(1) == "||")) + { + tok3->str(value); + ret = true; + } + + // Variable is used somehow in a non-defined pattern => bail out + if (tok3->varId() == varid) + { + // This is a really generic bailout so let's try to avoid this. + // There might be lots of false negatives. + if (_settings->debugwarnings) + { + // FIXME: Fix all the debug warnings for values and then + // remove this bailout + if (pointeralias) + break; + + // suppress debug-warning when calling member function + if (Token::Match(tok3->next(), ". %var% (")) + break; + + // suppress debug-warning when assignment + if (Token::simpleMatch(tok3->next(), "=")) + break; + + // taking address of variable.. + if (Token::Match(tok3->tokAt(-2), "return|= & %var% ;")) + break; + + // parameter in function call.. + if (Token::Match(tok3->tokAt(-2), "%var% ( %var% ,|)") || + Token::Match(tok3->previous(), ", %var% ,|)")) + break; + + // conditional increment + if (Token::Match(tok3->tokAt(-3), ") { ++|--") || + Token::Match(tok3->tokAt(-2), ") { %var% ++|--")) + break; + + std::list locationList; + ErrorLogger::ErrorMessage::FileLocation loc; + loc.line = tok3->linenr(); + loc.setfile(file(tok3)); + locationList.push_back(loc); + + const ErrorLogger::ErrorMessage errmsg(locationList, + Severity::debug, + "simplifyKnownVariables: bailing out (variable="+tok3->str()+", value="+value+")", + "debug"); + + if (_errorLogger) + _errorLogger->reportErr(errmsg); + else + Check::reportError(errmsg); + } + + break; + } + + // Using the variable in condition.. + if (Token::Match(tok3->previous(), ("if ( " + structname + " %varid% ==|!=|<|<=|>|>=|)").c_str(), varid) || + Token::Match(tok3, ("( " + structname + " %varid% ==|!=|<|<=|>|>=").c_str(), varid) || + Token::Match(tok3, ("!|==|!=|<|<=|>|>= " + structname + " %varid% ==|!=|<|<=|>|>=|)").c_str(), varid) || + Token::Match(tok3->previous(), "strlen|free ( %varid% )", varid)) + { + if (!structname.empty()) + { + tok3->deleteNext(); + tok3->deleteNext(); + } + tok3 = tok3->next(); + tok3->str(value); + tok3->varId(valueVarId); + ret = true; + } + + // Delete pointer alias + if (pointeralias && tok3->str() == "delete" && + (Token::Match(tok3, "delete %varid% ;", varid) || + Token::Match(tok3, "delete [ ] %varid%", varid))) + { + tok3 = (tok3->strAt(1) == "[") ? tok3->tokAt(3) : tok3->next(); + tok3->str(value); + tok3->varId(valueVarId); + ret = true; + } + + // Variable is used in function call.. + if (Token::Match(tok3, ("%var% ( " + structname + " %varid% ,").c_str(), varid)) + { + const char * const functionName[] = + { + "memcmp","memcpy","memmove","memset", + "strcmp","strcpy","strncpy","strdup" + }; + for (unsigned int i = 0; i < (sizeof(functionName) / sizeof(*functionName)); ++i) + { + if (tok3->str() == functionName[i]) + { + Token *par1 = tok3->next()->next(); + if (!structname.empty()) + { + par1->deleteThis(); + par1->deleteThis(); + } + par1->str(value); + par1->varId(valueVarId); + break; + } + } + } + + // Variable is used as 2nd parameter in function call.. + if (Token::Match(tok3, ("%var% ( %any% , " + structname + " %varid% ,|)").c_str(), varid)) + { + const char * const functionName[] = + { + "memcmp","memcpy","memmove", + "strcmp","strcpy","strncmp","strncpy" + }; + for (unsigned int i = 0; i < (sizeof(functionName) / sizeof(*functionName)); ++i) + { + if (tok3->str() == functionName[i]) + { + Token *par = tok3->tokAt(4); + if (!structname.empty()) + { + par->deleteThis(); + par->deleteThis(); + } + par->str(value); + par->varId(valueVarId); + break; + } + } + } + + // array usage + if (Token::Match(tok3, ("[(,] " + structname + " %varid% [+-*/[]").c_str(), varid)) + { + if (!structname.empty()) + { + tok3->deleteNext(); + tok3->deleteNext(); + } + tok3 = tok3->next(); + tok3->str(value); + tok3->varId(valueVarId); + ret = true; + } + + // Variable is used in calculation.. + if (((tok3->previous()->varId() > 0) && Token::Match(tok3, ("& " + structname + " %varid%").c_str(), varid)) || + Token::Match(tok3, ("[=+-*/[] " + structname + " %varid% [=?+-*/;])]").c_str(), varid) || + Token::Match(tok3, ("[(=+-*/[] " + structname + " %varid% <<|>>").c_str(), varid) || + Token::Match(tok3, ("<<|>> " + structname + " %varid% [+-*/;])]").c_str(), varid) || + Token::Match(tok3->previous(), ("[=+-*/[] ( " + structname + " %varid%").c_str(), varid)) + { + if (!structname.empty()) + { + tok3->deleteNext(); + tok3->deleteNext(); + } + tok3 = tok3->next(); + tok3->str(value); + tok3->varId(valueVarId); + if (tok3->previous()->str() == "*" && valueIsPointer) + { + tok3 = tok3->previous(); + tok3->deleteThis(); + } + ret = true; + } + + if (Token::simpleMatch(tok3, "= {")) + { + unsigned int indentlevel4 = 0; + for (const Token *tok4 = tok3; tok4; tok4 = tok4->next()) + { + if (tok4->str() == "{") + ++indentlevel4; + else if (tok4->str() == "}") + { + if (indentlevel4 <= 1) + break; + --indentlevel4; + } + if (Token::Match(tok4, "{|, %varid% ,|}", varid)) + { + tok4->next()->str(value); + tok4->next()->varId(valueVarId); + ret = true; + } + } + } + + // Using the variable in for-condition.. + if (Token::simpleMatch(tok3, "for (")) + { + for (Token *tok4 = tok3->tokAt(2); tok4; tok4 = tok4->next()) + { + if (tok4->str() == "(" || tok4->str() == ")") + break; + + // Replace variable used in condition.. + if (Token::Match(tok4, "; %var% <|<=|!= %var% ; ++| %var% ++| )")) + { + const Token *inctok = tok4->tokAt(5); + if (inctok->str() == "++") + inctok = inctok->next(); + if (inctok->varId() == varid) + break; + + if (tok4->next()->varId() == varid) + { + tok4->next()->str(value); + tok4->next()->varId(valueVarId); + ret = true; + } + if (tok4->tokAt(3)->varId() == varid) + { + tok4->tokAt(3)->str(value); + tok4->tokAt(3)->varId(valueVarId); + ret = true; + } + } + } + } + + if (indentlevel == indentlevel3 && Token::Match(tok3->next(), "%varid% ++|--", varid) && MathLib::isInt(value)) + { + const std::string op(tok3->strAt(2)); + if (Token::Match(tok3, "[{};] %any% %any% ;")) + { + Token::eraseTokens(tok3, tok3->tokAt(3)); + } + else + { + tok3 = tok3->next(); + tok3->str(value); + tok3->varId(valueVarId); + tok3->deleteNext(); + } + incdec(value, op); + if (!Token::simpleMatch((*tok2)->tokAt(-2), "for (")) + { + (*tok2)->tokAt(2)->str(value); + (*tok2)->tokAt(2)->varId(valueVarId); + } + ret = true; + } + + if (indentlevel == indentlevel3 && Token::Match(tok3->next(), "++|-- %varid%", varid) && MathLib::isInt(value) && + !Token::Match(tok3->tokAt(3), "[.[]")) + { + incdec(value, tok3->strAt(1)); + (*tok2)->tokAt(2)->str(value); + (*tok2)->tokAt(2)->varId(valueVarId); + if (Token::Match(tok3, "[;{}] %any% %any% ;")) + { + Token::eraseTokens(tok3, tok3->tokAt(3)); + } + else + { + tok3->deleteNext(); + tok3->next()->str(value); + tok3->next()->varId(valueVarId); + } + tok3 = tok3->next(); + ret = true; + } + + // return variable.. + if (Token::Match(tok3, "return %varid% %any%", varid) && + isOp(tok3->tokAt(2))) + { + tok3->next()->str(value); + tok3->next()->varId(valueVarId); + } + + else if (pointeralias && Token::Match(tok3, "return * %varid% ;", varid)) + { + tok3->deleteNext(); + tok3->next()->str(value); + tok3->next()->varId(valueVarId); + } + } + return ret; +} + void Tokenizer::elseif() { diff --git a/lib/tokenize.h b/lib/tokenize.h index c98e46456..913d2ceed 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -308,6 +308,12 @@ public: */ bool simplifyKnownVariables(); + /** + * utility function for simplifyKnownVariables. Perform simplification + * of a given variable + */ + bool simplifyKnownVariablesSimplify(Token **tok2, Token *tok3, unsigned int varid, const std::string &structname, std::string &value, unsigned int valueVarId, bool valueIsPointer, bool pointeralias, int indentlevel); + /** Replace a "goto" with the statements */ void simplifyGoto(); From 2525b9db40cfaf4ae2dcc921de5c760a2d06ba21 Mon Sep 17 00:00:00 2001 From: Reijo Tomperi Date: Fri, 11 Feb 2011 23:37:38 +0200 Subject: [PATCH 10/17] Add check to runastyle to enforce correct astyle version usage. --- runastyle | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/runastyle b/runastyle index 4da7b7866..812c3f780 100755 --- a/runastyle +++ b/runastyle @@ -1,4 +1,9 @@ #!/bin/bash +ASTYLE_VERSION="Artistic Style Version 2.01" +if [ "`astyle --version 2>&1`" != "${ASTYLE_VERSION}" ]; then + echo "You should use: ${ASTYLE_VERSION}"; + exit 1; +fi style="--style=ansi --lineend=linux --min-conditional-indent=0" options="--pad-header --unpad-paren --suffix=none" From f5ed52b84b437c1494e331ba05c2d5f92d6960c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Debrard?= Date: Fri, 11 Feb 2011 23:38:23 +0100 Subject: [PATCH 11/17] fix #2569 check postfix increment on boolean --- lib/checkother.cpp | 32 ++++++++++++++++++++++++++++++++ lib/checkother.h | 7 +++++++ test/testother.cpp | 20 ++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 1e94d1b33..799fadbcf 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -34,6 +34,38 @@ CheckOther instance; //--------------------------------------------------------------------------- +void CheckOther::checkIncrementBoolean() +{ + if (!_settings->_checkCodingStyle) + return; + + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "%var% ++")) + { + if (tok->varId()) + { + const Token *declTok = Token::findmatch(_tokenizer->tokens(), "bool %varid%", tok->varId()); + if (declTok) + incrementBooleanError(tok); + } + } + } +} + +void CheckOther::incrementBooleanError(const Token *tok) +{ + reportError( + tok, + Severity::style, + "incrementboolean", + "The use of a variable of type bool with the ++ postfix operator is always true and deprecated by the C++ Standard.\n" + "The operand of a postfix increment operator may be of type bool but it is deprecated by C++ Standard (Annex D-1) and the operand is always set to true\n" + ); +} + +//--------------------------------------------------------------------------- + void CheckOther::clarifyCalculation() { diff --git a/lib/checkother.h b/lib/checkother.h index 07df64867..cb0266cc1 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -88,6 +88,7 @@ public: checkOther.checkCatchExceptionByValue(); checkOther.checkMemsetZeroBytes(); checkOther.checkIncorrectStringCompare(); + checkOther.checkIncrementBoolean(); } /** @brief Clarify calculation for ".. a * b ? .." */ @@ -184,6 +185,9 @@ public: /** @brief %Check for using bad usage of strncmp and substr */ void checkIncorrectStringCompare(); + /** @brief %Check for using postfix increment on bool */ + void checkIncrementBoolean(); + // Error messages.. void cstyleCastError(const Token *tok); void dangerousUsageStrtolError(const Token *tok); @@ -209,6 +213,7 @@ public: void memsetZeroBytesError(const Token *tok, const std::string &varname); void sizeofForArrayParameterError(const Token *tok); void incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string, const std::string &len); + void incrementBooleanError(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -249,6 +254,7 @@ public: c.memsetZeroBytesError(0, "varname"); c.clarifyCalculationError(0); c.incorrectStringCompareError(0, "substr", "\"Hello World\"", "12"); + c.incrementBooleanError(0); } std::string myName() const @@ -289,6 +295,7 @@ public: "* mutual exclusion over || always evaluating to true\n" "* exception caught by value instead of by reference\n" "* Clarify calculation with parantheses\n" + "* using increment on boolean\n" // optimisations "* optimisation: detect post increment/decrement\n"; diff --git a/test/testother.cpp b/test/testother.cpp index f5adf548d..1e7eca1d3 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -104,6 +104,8 @@ private: TEST_CASE(clarifyCalculation); TEST_CASE(incorrectStringCompare); + + TEST_CASE(incrementBoolean); } void check(const char code[], const char *filename = NULL) @@ -141,6 +143,7 @@ private: checkOther.checkMemsetZeroBytes(); checkOther.clarifyCalculation(); checkOther.checkIncorrectStringCompare(); + checkOther.checkIncrementBoolean(); } @@ -1892,6 +1895,23 @@ private: ASSERT_EQUALS("", errout.str()); } + + void incrementBoolean() + { + check("bool bValue = true;\n" + "bValue++;\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) The use of a variable of type bool with the ++ postfix operator is always true and deprecated by the C++ Standard.\n", errout.str()); + + check("void f(bool test){\n" + " test++;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) The use of a variable of type bool with the ++ postfix operator is always true and deprecated by the C++ Standard.\n", errout.str()); + + check("void f(int test){\n" + " test++;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestOther) From 78b5361ec843fef3746ffa19217d534e11afe4cf Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Fri, 11 Feb 2011 19:09:24 -0500 Subject: [PATCH 12/17] fix #2568 (False positive: (style) Union 'A_t' hides typedef with same name (forward declaration)) --- lib/tokenize.cpp | 19 +++++++++++++++---- lib/tokenize.h | 2 +- test/testsimplifytokens.cpp | 13 ++++++++++++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 00c61dcad..b42b829d2 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -509,7 +509,7 @@ void Tokenizer::duplicateDeclarationError(const Token *tok1, const Token *tok2, } // check if this statement is a duplicate definition -bool Tokenizer::duplicateTypedef(Token **tokPtr, const Token *name) +bool Tokenizer::duplicateTypedef(Token **tokPtr, const Token *name, const Token *typeDef) { // check for an end of definition const Token * tok = *tokPtr; @@ -624,7 +624,11 @@ bool Tokenizer::duplicateTypedef(Token **tokPtr, const Token *name) int level = (tok->previous()->str() == "}") ? 1 : 0; while (tok && tok->previous() && (!Token::Match(tok->previous(), ";|{") || (level != 0))) { - if (tok->previous()->str() == "typedef") + if (tok->previous()->str() == "}") + { + tok = tok->previous()->link(); + } + else if (tok->previous()->str() == "typedef") { duplicateTypedefError(*tokPtr, name, "Typedef"); return true; @@ -636,7 +640,14 @@ bool Tokenizer::duplicateTypedef(Token **tokPtr, const Token *name) } else if (tok->previous()->str() == "struct") { - if (tok->next()->str() != ";") + if (tok->strAt(-2) == "typedef" && + tok->next()->str() == "{" && + typeDef->strAt(3) != "{") + { + // declaration after forward declaration + return true; + } + else if (tok->next()->str() != ";") { duplicateTypedefError(*tokPtr, name, "Struct"); return true; @@ -1373,7 +1384,7 @@ void Tokenizer::simplifyTypedef() { tok2 = tok2->next(); } - else if (duplicateTypedef(&tok2, typeName)) + else if (duplicateTypedef(&tok2, typeName, typeDef)) { exitScope = scope; diff --git a/lib/tokenize.h b/lib/tokenize.h index 913d2ceed..179b6a406 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -580,7 +580,7 @@ public: */ void duplicateEnumError(const Token *tok1, const Token *tok2, const std::string & type); - bool duplicateTypedef(Token **tokPtr, const Token *name); + bool duplicateTypedef(Token **tokPtr, const Token *name, const Token *typeDef); void duplicateTypedefError(const Token *tok1, const Token *tok2, const std::string & type); /** diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index fe8d2b8a9..902b4567f 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -235,6 +235,7 @@ private: TEST_CASE(simplifyTypedef75); // ticket #2426 TEST_CASE(simplifyTypedef76); // ticket #2453 TEST_CASE(simplifyTypedef77); // ticket #2554 + TEST_CASE(simplifyTypedef78); // ticket #2568 TEST_CASE(simplifyTypedefFunction1); TEST_CASE(simplifyTypedefFunction2); // ticket #1685 @@ -3941,7 +3942,7 @@ private: ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:1]: (style) Typedef 'A' hides typedef with same name\n" "[test.cpp:20] -> [test.cpp:1]: (style) Function parameter 'A' hides typedef with same name\n" "[test.cpp:21] -> [test.cpp:1]: (style) Variable 'A' hides typedef with same name\n" - "[test.cpp:24] -> [test.cpp:1]: (style) Struct 'A' hides typedef with same name\n", errout.str()); + "[test.cpp:24] -> [test.cpp:1]: (style) Typedef 'A' hides typedef with same name\n", errout.str()); } void simplifyTypedef36() @@ -4846,6 +4847,16 @@ private: ASSERT_EQUALS(expected, sizeof_(code)); } + void simplifyTypedef78() // ticket #2568 + { + const char code[] = "typedef struct A A_t;\n" + "A_t a;\n" + "typedef struct A { } A_t;\n" + "A_t a1;\n"; + const std::string expected = "; struct A a ; struct A { } ; struct A a1 ;"; + ASSERT_EQUALS(expected, sizeof_(code)); + } + void simplifyTypedefFunction1() { { From 27febb062bb18457d3bfd4510fb185c33eb8b5cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 12 Feb 2011 08:06:59 +0100 Subject: [PATCH 13/17] cppcheck: Added HAVE_DEPENDENCIES define. Cppcheck cli can be compiled without dependencies. --- Makefile | 2 +- cli/cli.pro | 1 + cli/cmdlineparser.cpp | 4 ++++ lib/cppcheck.cpp | 4 ++-- tools/dmake.cpp | 3 ++- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 59988cc63..6dfbe39d4 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # This file is generated by tools/dmake, do not edit. ifndef CXXFLAGS - CXXFLAGS=-Wall -Wextra -Wshadow -pedantic -Wno-long-long -Wfloat-equal -Wcast-qual -Wsign-conversion -g + CXXFLAGS=-DHAVE_DEPENDENCIES -Wall -Wextra -Wshadow -pedantic -Wno-long-long -Wfloat-equal -Wcast-qual -Wsign-conversion -g endif ifndef CXX diff --git a/cli/cli.pro b/cli/cli.pro index e04e4e158..2195098de 100644 --- a/cli/cli.pro +++ b/cli/cli.pro @@ -5,6 +5,7 @@ INCLUDEPATH += . ../lib OBJECTS_DIR = temp CONFIG += warn_on CONFIG -= qt app_bundle +DEFINES += HAVE_DEPENDENCIES BASEPATH = ../externals/tinyxml/ include($$PWD/../externals/tinyxml/tinyxml.pri) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index d89329e76..d8f61bab5 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -28,8 +28,10 @@ #include "path.h" #include "filelister.h" +#ifdef HAVE_DEPENDENCIES // xml is used in rules #include +#endif static void AddFilesToList(const std::string& FileList, std::vector& PathNames) { @@ -503,6 +505,7 @@ bool CmdLineParser::ParseFromArgs(int argc, const char* const argv[]) _settings->_showtime = SHOWTIME_NONE; } +#ifdef HAVE_DEPENDENCIES // Rule given at command line else if (strncmp(argv[i], "--rule=", 7) == 0) { @@ -549,6 +552,7 @@ bool CmdLineParser::ParseFromArgs(int argc, const char* const argv[]) } } } +#endif // Print help else if (strcmp(argv[i], "-h") == 0 || strcmp(argv[i], "--help") == 0) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 7530498a7..9f3d81320 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -30,7 +30,7 @@ #include #include "timer.h" -#ifndef __BORLANDC__ +#ifdef HAVE_DEPENDENCIES #define PCRE_STATIC #include #endif @@ -309,7 +309,7 @@ void CppCheck::checkFile(const std::string &code, const char FileName[]) (*it)->runSimplifiedChecks(&_tokenizer, &_settings, this); } -#ifndef __BORLANDC__ +#ifdef HAVE_DEPENDENCIES // Are there extra rules? if (!_settings.rules.empty()) { diff --git a/tools/dmake.cpp b/tools/dmake.cpp index ea7cdcb34..88aa47159 100644 --- a/tools/dmake.cpp +++ b/tools/dmake.cpp @@ -225,7 +225,7 @@ int main(int argc, char **argv) // Makefile settings.. if (release) { - makeConditionalVariable(fout, "CXXFLAGS", "-O2 -DNDEBUG -Wall"); + makeConditionalVariable(fout, "CXXFLAGS", "-O2 -DNDEBUG -DHAVE_DEPENDENCIES -Wall"); } else { @@ -235,6 +235,7 @@ int main(int argc, char **argv) // The _GLIBCXX_DEBUG doesn't work in cygwin makeConditionalVariable(fout, "CXXFLAGS", + "-DHAVE_DEPENDENCIES " "-Wall " "-Wextra " "-Wshadow " From edc472dd343ef7d102453a4a1ed0d236525dd119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 12 Feb 2011 08:50:38 +0100 Subject: [PATCH 14/17] readme: show how cppcheck is compiled with g++ with or without dependencies --- readme.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/readme.txt b/readme.txt index 170559978..8788f47aa 100644 --- a/readme.txt +++ b/readme.txt @@ -44,8 +44,11 @@ Compiling g++ (for experts) ================= - If you just want to build Cppcheck then you can use this command: - g++ -o cppcheck -lpcre -Ilib -Iexternals cli/*.cpp lib/*.cpp externals/tinyxml/*.cpp + If you just want to build Cppcheck without dependencies then you can use this command: + g++ -o cppcheck -Ilib cli/*.cpp lib/*.cpp + + If you want to use --rule and --rule-file then dependencies are needed: + g++ -o cppcheck -lpcre -DHAVE_DEPENDENCIES -Ilib -Iexternals cli/*.cpp lib/*.cpp externals/tinyxml/*.cpp mingw ===== make LDFLAGS=-lshlwapi From 63ade3e4f6721cb45eeb0ed78156f034fc992d3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 12 Feb 2011 09:24:20 +0100 Subject: [PATCH 15/17] Tokenizer::simplifyKnownVariables: Split up the function into smaller functions. Broke out ..GetData function that extracts info about assigned variable before the simplification is made. --- lib/tokenize.cpp | 153 +++++++++++++++++++++++++---------------------- lib/tokenize.h | 6 ++ 2 files changed, 89 insertions(+), 70 deletions(-) diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index b42b829d2..352363f67 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -6237,77 +6237,9 @@ bool Tokenizer::simplifyKnownVariables() Token *tok3 = NULL; bool valueIsPointer = false; - if (Token::Match(tok2->tokAt(-2), "for ( %varid% = %num% ; %varid% <|<= %num% ; ++| %varid% ++| ) {", varid)) - { - // is there a "break" in the for loop? - bool hasbreak = false; - unsigned int indentlevel4 = 0; // indentlevel for tok4 - for (const Token *tok4 = tok2->previous()->link(); tok4; tok4 = tok4->next()) - { - if (tok4->str() == "{") - ++indentlevel4; - else if (tok4->str() == "}") - { - if (indentlevel4 <= 1) - break; - --indentlevel4; - } - else if (tok4->str() == "break") - { - hasbreak = true; - break; - } - } - if (hasbreak) - break; + if (!simplifyKnownVariablesGetData(varid, &tok2, &tok3, value, valueVarId, valueIsPointer, floatvars.find(tok2->varId()) != floatvars.end())) + continue; - // no break => the value of the counter value is known after the for loop.. - const std::string compareop = tok2->strAt(5); - if (compareop == "<") - { - value = tok2->strAt(6); - valueVarId = tok2->tokAt(6)->varId(); - } - else - value = MathLib::toString(MathLib::toLongNumber(tok2->strAt(6)) + 1); - - // Skip for-body.. - tok3 = tok2->previous()->link()->next()->link()->next(); - } - else - { - value = tok2->strAt(2); - valueVarId = tok2->tokAt(2)->varId(); - if (Token::simpleMatch(tok2->next(), "[")) - { - value = tok2->next()->link()->strAt(2); - valueVarId = 0; - } - else if (value == "&") - { - value = tok2->strAt(3); - valueVarId = tok2->tokAt(3)->varId(); - - // *ptr = &var; *ptr = 5; - // equals - // var = 5; not *var = 5; - if (tok2->strAt(4) == ";") - valueIsPointer = true; - } - - // float value should contain a "." - else if (tok2->tokAt(2)->isNumber() && - floatvars.find(tok2->varId()) != floatvars.end() && - value.find(".") == std::string::npos) - { - value += ".0"; - } - - if (Token::simpleMatch(tok2->next(), "= &")) - tok2 = tok2->tokAt(3); - - tok3 = tok2->next(); - } ret |= simplifyKnownVariablesSimplify(&tok2, tok3, varid, structname, value, valueVarId, valueIsPointer, pointeralias, indentlevel); } } @@ -6319,6 +6251,87 @@ bool Tokenizer::simplifyKnownVariables() return ret; } +bool Tokenizer::simplifyKnownVariablesGetData(unsigned int varid, Token **_tok2, Token **_tok3, std::string &value, unsigned int &valueVarId, bool &valueIsPointer, bool floatvar) +{ + Token *tok2 = *_tok2; + Token *tok3 = *_tok3; + + if (Token::Match(tok2->tokAt(-2), "for ( %varid% = %num% ; %varid% <|<= %num% ; ++| %varid% ++| ) {", varid)) + { + // is there a "break" in the for loop? + bool hasbreak = false; + unsigned int indentlevel4 = 0; // indentlevel for tok4 + for (const Token *tok4 = tok2->previous()->link(); tok4; tok4 = tok4->next()) + { + if (tok4->str() == "{") + ++indentlevel4; + else if (tok4->str() == "}") + { + if (indentlevel4 <= 1) + break; + --indentlevel4; + } + else if (tok4->str() == "break") + { + hasbreak = true; + break; + } + } + if (hasbreak) + return false; + + // no break => the value of the counter value is known after the for loop.. + const std::string compareop = tok2->strAt(5); + if (compareop == "<") + { + value = tok2->strAt(6); + valueVarId = tok2->tokAt(6)->varId(); + } + else + value = MathLib::toString(MathLib::toLongNumber(tok2->strAt(6)) + 1); + + // Skip for-body.. + tok3 = tok2->previous()->link()->next()->link()->next(); + } + else + { + value = tok2->strAt(2); + valueVarId = tok2->tokAt(2)->varId(); + if (Token::simpleMatch(tok2->next(), "[")) + { + value = tok2->next()->link()->strAt(2); + valueVarId = 0; + } + else if (value == "&") + { + value = tok2->strAt(3); + valueVarId = tok2->tokAt(3)->varId(); + + // *ptr = &var; *ptr = 5; + // equals + // var = 5; not *var = 5; + if (tok2->strAt(4) == ";") + valueIsPointer = true; + } + + // float value should contain a "." + else if (tok2->tokAt(2)->isNumber() && + floatvar && + value.find(".") == std::string::npos) + { + value += ".0"; + } + + if (Token::simpleMatch(tok2->next(), "= &")) + tok2 = tok2->tokAt(3); + + tok3 = tok2->next(); + } + *_tok2 = tok2; + *_tok3 = tok3; + return true; +} + bool Tokenizer::simplifyKnownVariablesSimplify(Token **tok2, Token *tok3, unsigned int varid, const std::string &structname, std::string &value, unsigned int valueVarId, bool valueIsPointer, bool pointeralias, int indentlevel) { bool ret = false;; diff --git a/lib/tokenize.h b/lib/tokenize.h index 179b6a406..d70a03c76 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -308,6 +308,12 @@ public: */ bool simplifyKnownVariables(); + /** + * Utility function for simplifyKnownVariables. Get data about an + * assigned variable. + */ + bool simplifyKnownVariablesGetData(unsigned int varid, Token **_tok2, Token **_tok3, std::string &value, unsigned int &valueVarId, bool &valueIsPointer, bool floatvar); + /** * utility function for simplifyKnownVariables. Perform simplification * of a given variable From d8119cd57a6547648b3df5c66d483d25ad33bb82 Mon Sep 17 00:00:00 2001 From: Raphael Geissert Date: Sat, 12 Feb 2011 02:42:31 -0600 Subject: [PATCH 16/17] Fix test for architectures where char is unsigned --- test/testbufferoverrun.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 189e4364b..e065d2e87 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -887,8 +887,8 @@ private: " a[-1] = 0;\n" // negative index " a[256] = 0;\n" // 256 > CHAR_MAX "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (error) Array 'a[256]' index -1 out of bounds\n" - "[test.cpp:4]: (error) Array 'a[256]' index 256 out of bounds\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (error) Array 'a[256]' index 256 out of bounds\n" + "[test.cpp:3]: (error) Array 'a[256]' index -1 out of bounds\n", errout.str()); } check("void f(signed char n) {\n" From 318f2e8a5733adb2e863dd229b8102bd6c4e2d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sat, 12 Feb 2011 11:31:10 +0100 Subject: [PATCH 17/17] Fixed #2561 (False positive on array index when using conditional operator) --- lib/checkbufferoverrun.cpp | 5 +++++ test/testbufferoverrun.cpp | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 0df90e43d..a0f417975 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -464,6 +464,11 @@ void CheckBufferOverrun::parse_for_body(const Token *tok2, const ArrayInfo &arra break; } + // TODO: try to reduce false negatives. This is just a quick fix + // for TestBufferOverrun::array_index_for_question + if (tok2->str() == "?") + break; + if (Token::Match(tok2, "if|switch")) { if (bailoutIfSwitch(tok2, arrayInfo.varid)) diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index e065d2e87..d7137ee35 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -116,6 +116,7 @@ private: TEST_CASE(array_index_for_break); // FP: for,break TEST_CASE(array_index_for); // FN: for,if TEST_CASE(array_index_for_neq); // #2211: Using != in condition + TEST_CASE(array_index_for_question); // #2561: for, ?: TEST_CASE(buffer_overrun_1); TEST_CASE(buffer_overrun_2); @@ -1381,6 +1382,18 @@ private: ASSERT_EQUALS("[test.cpp:4]: (error) Buffer access out-of-bounds: a\n", errout.str()); } + void array_index_for_question() + { + // Ticket #2561 - using ?: inside for loop + check("void f() {\n" + " int a[10];\n" + " for (int i = 0; i != 10; ++i) {\n" + " i == 0 ? 0 : a[i-1];\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } + void buffer_overrun_1() { check("void f()\n"