From 1373e14bc933f6b4896a4d4742031945ee427efe Mon Sep 17 00:00:00 2001 From: Reijo Tomperi Date: Mon, 9 Feb 2009 20:51:04 +0000 Subject: [PATCH] Fix ticket #93 (Write xml results into error stream instead of results.xml file.) and also refactor the code to use ErrorLogger::reportErr() for all errors, for both xml and plain text. And move xml formatting from Cppcheck to CppcheckExecutor. --- man/cppcheck.1.xml | 6 ++-- src/checkdangerousfunctions.cpp | 16 ++++----- src/checkheaders.cpp | 16 +++++++-- src/cppcheck.cpp | 41 ++++++++---------------- src/cppcheck.h | 11 ++++--- src/cppcheckexecutor.cpp | 39 +++++++++++++++++++--- src/cppcheckexecutor.h | 22 +++++++------ src/errorlogger.cpp | 33 ++++++++++++------- src/errorlogger.h | 57 +++++++++++++++++++++++++-------- src/tokenize.cpp | 5 ++- src/tokenize.h | 2 ++ test/testdangerousfunctions.cpp | 6 ++-- test/testsuite.cpp | 12 +++---- test/testsuite.h | 3 +- tools/errmsg.cpp | 37 +++++++++++++-------- 15 files changed, 196 insertions(+), 110 deletions(-) diff --git a/man/cppcheck.1.xml b/man/cppcheck.1.xml index 8ada3c968..7302b06ee 100644 --- a/man/cppcheck.1.xml +++ b/man/cppcheck.1.xml @@ -110,7 +110,7 @@ man(1), man(7), http://www.tldp.org/HOWTO/Man-Page/ - + @@ -187,9 +187,9 @@ files, this is not needed. - + - Write results in results.xml + Write results in xml to error stream diff --git a/src/checkdangerousfunctions.cpp b/src/checkdangerousfunctions.cpp index 010691ca8..f97dc2f53 100644 --- a/src/checkdangerousfunctions.cpp +++ b/src/checkdangerousfunctions.cpp @@ -56,17 +56,17 @@ void CheckDangerousFunctionsClass::dangerousFunctions() { for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { - if (Token::Match(tok, "mktemp (")) + if (Token::simpleMatch(tok, "mktemp (")) { - std::ostringstream ostr; - ostr << _tokenizer->fileLine(tok) << ": Found 'mktemp'. You should use 'mkstemp' instead"; - _errorLogger->reportErr(ostr.str()); + _errorLogger->dangerousFunctionmktemp(_tokenizer, tok); } - else if (Token::Match(tok, "gets|scanf (")) + else if (Token::simpleMatch(tok, "gets (")) { - std::ostringstream ostr; - ostr << _tokenizer->fileLine(tok) << ": Found '" << tok->str() << "'. You should use 'fgets' instead"; - _errorLogger->reportErr(ostr.str()); + _errorLogger->dangerousFunctiongets(_tokenizer, tok); + } + else if (Token::simpleMatch(tok, "scanf (")) + { + _errorLogger->dangerousFunctionscanf(_tokenizer, tok); } } } diff --git a/src/checkheaders.cpp b/src/checkheaders.cpp index fe22a636c..fdee94113 100644 --- a/src/checkheaders.cpp +++ b/src/checkheaders.cpp @@ -60,7 +60,13 @@ void CheckHeaders::WarningHeaderWithImplementation() { std::ostringstream ostr; ostr << _tokenizer->fileLine(tok) << ": Found implementation in header"; - _errorLogger->reportErr(ostr.str()); + + // TODO, this check is currently not used, but if it is some day + // it should give correct id and severity by calling proper function + // from errorLogger. It should not call reportErr directly. + std::list empty; + empty.push_back(FileLocation()); + _errorLogger->reportErr(empty, "id", "severity", ostr.str()); // Goto next file.. unsigned int fileindex = tok->fileIndex(); @@ -248,7 +254,13 @@ void CheckHeaders::WarningIncludeHeader() ostr << _tokenizer->fileLine(includetok) << ": The included header '" << includefile << "' is not needed"; if (NeedDeclaration) ostr << " (but a forward declaration is needed)"; - _errorLogger->reportErr(ostr.str()); + + // TODO, this check is currently not used, but if it is some day + // it should give correct id and severity by calling proper function + // from errorLogger. It should not call reportErr directly. + std::list empty; + empty.push_back(FileLocation()); + _errorLogger->reportErr(empty, "id", "severity", ostr.str()); // TODO } } } diff --git a/src/cppcheck.cpp b/src/cppcheck.cpp index 953d8430a..1ffa56694 100644 --- a/src/cppcheck.cpp +++ b/src/cppcheck.cpp @@ -186,7 +186,7 @@ std::string CppCheck::parseFromArgs(int argc, const char* const argv[]) " -s, --style Check coding style\n" " --unused-functions Check if there are unused functions\n" " -v, --verbose More detailed error reports\n" - " --xml Write results in results.xml\n" + " --xml Write results in xml to error stream.\n" "\n" "Example usage:\n" " # Recursively check the current folder. Print the progress on the screen and\n" @@ -284,15 +284,7 @@ unsigned int CppCheck::check() _checkFunctionUsage.check(); } - if (_settings._xml) - { - std::ofstream fxml("result.xml"); - fxml << "\n"; - fxml << "\n"; - for (std::list::const_iterator it = _xmllist.begin(); it != _xmllist.end(); ++it) - fxml << " " << *it << "\n"; - fxml << ""; - } + unsigned int result = static_cast(_errorList.size()); _errorList.clear(); @@ -416,10 +408,20 @@ void CppCheck::checkFile(const std::string &code, const char FileName[]) if (ErrorLogger::strPlusChar()) checkOther.strPlusChar(); } + +Settings CppCheck::settings() const +{ + return _settings; +} + //--------------------------------------------------------------------------- -void CppCheck::reportErr(const std::string &errmsg) +void CppCheck::reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg) { + std::ostringstream text; + text << ErrorLogger::callStackToString(callStack) << ": (" << severity << ") " << msg; + std::string errmsg = text.str(); + // Alert only about unique errors if (std::find(_errorList.begin(), _errorList.end(), errmsg) != _errorList.end()) return; @@ -431,8 +433,7 @@ void CppCheck::reportErr(const std::string &errmsg) errmsg2 += "\n Defines=\'" + cfg + "\'\n"; } - - _errorLogger->reportErr(errmsg2); + _errorLogger->reportErr(callStack, id, severity, msg); _errout << errmsg2 << std::endl; } @@ -442,17 +443,3 @@ void CppCheck::reportOut(const std::string & /*outmsg*/) // This is currently never called. It is here just to comply with // the interface. } - -void CppCheck::reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg) -{ - std::ostringstream xml; - xml << ""; - - _xmllist.push_back(xml.str()); -} diff --git a/src/cppcheck.h b/src/cppcheck.h index 237b9b805..67df942bd 100644 --- a/src/cppcheck.h +++ b/src/cppcheck.h @@ -63,6 +63,12 @@ public: */ void settings(const Settings &settings); + /** + * Get copy of current settings. + * @return a copy of current settings + */ + Settings settings() const; + /** * Add new file to be checked. * @@ -104,7 +110,7 @@ private: * "[filepath:line number] Message", e.g. * "[main.cpp:4] Uninitialized member variable" */ - virtual void reportErr(const std::string &errmsg); + virtual void reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg); /** * Information about progress is directed here. @@ -113,9 +119,6 @@ private: */ virtual void reportOut(const std::string &outmsg); - /** xml output of errors */ - virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg); - std::list _errorList; std::ostringstream _errout; Settings _settings; diff --git a/src/cppcheckexecutor.cpp b/src/cppcheckexecutor.cpp index ed43e98e9..090625306 100644 --- a/src/cppcheckexecutor.cpp +++ b/src/cppcheckexecutor.cpp @@ -24,7 +24,7 @@ CppCheckExecutor::CppCheckExecutor() { - //ctor + _useXML = false; } CppCheckExecutor::~CppCheckExecutor() @@ -38,7 +38,20 @@ unsigned int CppCheckExecutor::check(int argc, const char* const argv[]) std::string result = cppCheck.parseFromArgs(argc, argv); if (result.length() == 0) { - return cppCheck.check(); + if (cppCheck.settings()._xml) + { + _useXML = true; + reportErr(""); + reportErr(""); + } + + unsigned int returnValue = cppCheck.check(); + if (_useXML) + { + reportErr(""); + } + + return returnValue; } else { @@ -57,7 +70,25 @@ void CppCheckExecutor::reportOut(const std::string &outmsg) std::cout << outmsg << std::endl; } -void CppCheckExecutor::reportXml(const std::string & /*file*/, const std::string & /*line*/, const std::string & /*id*/, const std::string & /*severity*/, const std::string & /*msg*/) +void CppCheckExecutor::reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg) { - // never used + if (_useXML) + { + std::ostringstream xml; + xml << ""; + reportErr(xml.str()); + } + else + { + std::ostringstream text; + + text << ErrorLogger::callStackToString(callStack) << ": (" << severity << ") " << msg; + reportErr(text.str()); + } } diff --git a/src/cppcheckexecutor.h b/src/cppcheckexecutor.h index 1e6e0b220..190fac528 100644 --- a/src/cppcheckexecutor.h +++ b/src/cppcheckexecutor.h @@ -54,15 +54,7 @@ public: */ unsigned int check(int argc, const char* const argv[]); - /** - * Errors and warnings are directed here. This should be - * called by the CppCheck class only. - * - * @param errmsg Errors messages are normally in format - * "[filepath:line number] Message", e.g. - * "[main.cpp:4] Uninitialized member variable" - */ - virtual void reportErr(const std::string &errmsg); + /** * Information about progress is directed here. This should be @@ -73,7 +65,17 @@ public: virtual void reportOut(const std::string &outmsg); /** xml output of errors */ - virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg); + virtual void reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg); + +private: + + /** + * Helper function to print out errors. Appends a line change. + * @param errmsg, string to printed to error stream + */ + void reportErr(const std::string &errmsg); + + bool _useXML; }; #endif // CPPCHECKEXECUTOR_H diff --git a/src/errorlogger.cpp b/src/errorlogger.cpp index 0b352de80..74c3ac16b 100644 --- a/src/errorlogger.cpp +++ b/src/errorlogger.cpp @@ -32,20 +32,19 @@ void ErrorLogger::_writemsg(const Tokenizer *tokenizer, const Token *tok, const void ErrorLogger::_writemsg(const Tokenizer *tokenizer, const std::list &callstack, const char severity[], const std::string msg, const std::string &id) { - // Todo.. callstack handling - const std::string &file(tokenizer->getFiles()->at(callstack.back()->fileIndex())); - std::ostringstream linenr; - linenr << callstack.back()->linenr(); - reportXml(file, - linenr.str(), + std::list locationList; + for (std::list::const_iterator tok = callstack.begin(); tok != callstack.end(); ++tok) + { + FileLocation loc; + loc.file = tokenizer->file(*tok); + loc.line = (*tok)->linenr(); + locationList.push_back(loc); + } + + reportErr(locationList, id, severity, msg); - - std::ostringstream ostr; - for (std::list::const_iterator tok = callstack.begin(); tok != callstack.end(); ++tok) - ostr << (tok == callstack.begin() ? "" : " -> ") << tokenizer->fileLine(*tok); - reportErr(ostr.str() + ": (" + severity + ") " + msg); } @@ -57,5 +56,15 @@ void ErrorLogger::_writemsg(const std::string msg, const std::string &id) xml << " msg=\"" << msg << "\""; xml << ">"; - reportErr(msg); + std::list empty; + empty.push_back(FileLocation()); + reportErr(empty, "id", "severity", msg); +} + +std::string ErrorLogger::callStackToString(const std::list &callStack) +{ + std::ostringstream ostr; + for (std::list::const_iterator tok = callStack.begin(); tok != callStack.end(); ++tok) + ostr << (tok == callStack.begin() ? "" : " -> ") << "[" << (*tok).file << ":" << (*tok).line << "]"; + return ostr.str(); } diff --git a/src/errorlogger.h b/src/errorlogger.h index 8b3f89535..a5a6cb96d 100644 --- a/src/errorlogger.h +++ b/src/errorlogger.h @@ -27,6 +27,16 @@ class Token; class Tokenizer; +/** + * File name and line number. + */ +class FileLocation +{ +public: + std::string file; + unsigned int line; +}; + /** * This is an interface, which the class responsible of error logging * should implement. @@ -38,15 +48,6 @@ public: ErrorLogger() { } virtual ~ErrorLogger() { } - /** - * Errors and warnings are directed here. - * - * @param errmsg Errors messages are normally in format - * "[filepath:line number] Message", e.g. - * "[main.cpp:4] Uninitialized member variable" - */ - virtual void reportErr(const std::string &errmsg) = 0; - /** * Information about progress is directed here. * @@ -55,16 +56,17 @@ public: virtual void reportOut(const std::string &outmsg) = 0; /** - * XML output of error / warning + * Output of error / warning * Todo: callstack handling * - * @param file filepath (can be "") - * @param line line (can be "") + * @param callStack File names and line numbers where the error + * was found and where it was called from. Error location is last + * element in the list * @param id error id (function name) * @param severity severity of error (always, all, style, all+style, never) * @param msg error message in plain text */ - virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg) = 0; + virtual void reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg) = 0; void arrayIndexOutOfBounds(const Tokenizer *tokenizer, const std::list &Location) { @@ -381,6 +383,35 @@ public: return true; } + void dangerousFunctionmktemp(const Tokenizer *tokenizer, const Token *Location) + { + _writemsg(tokenizer, Location, "style", "Found 'mktemp'. You should use 'mkstemp' instead", "dangerousFunctionmktemp"); + } + static bool dangerousFunctionmktemp(const Settings &s) + { + return s._checkCodingStyle; + } + + void dangerousFunctiongets(const Tokenizer *tokenizer, const Token *Location) + { + _writemsg(tokenizer, Location, "style", "Found 'gets'. You should use 'fgets' instead", "dangerousFunctiongets"); + } + static bool dangerousFunctiongets(const Settings &s) + { + return s._checkCodingStyle; + } + + void dangerousFunctionscanf(const Tokenizer *tokenizer, const Token *Location) + { + _writemsg(tokenizer, Location, "style", "Found 'scanf'. You should use 'fgets' instead", "dangerousFunctionscanf"); + } + static bool dangerousFunctionscanf(const Settings &s) + { + return s._checkCodingStyle; + } + + + static std::string callStackToString(const std::list &callStack); private: void _writemsg(const Tokenizer *tokenizer, const Token *tok, const char severity[], const std::string msg, const std::string &id); diff --git a/src/tokenize.cpp b/src/tokenize.cpp index a97a757fa..a15579d5e 100644 --- a/src/tokenize.cpp +++ b/src/tokenize.cpp @@ -1740,6 +1740,9 @@ std::string Tokenizer::fileLine(const Token *tok) const return ostr.str(); } - +std::string Tokenizer::file(const Token *tok) const +{ + return _files.at(tok->fileIndex()); +} //--------------------------------------------------------------------------- diff --git a/src/tokenize.h b/src/tokenize.h index 525b675aa..62264d3fe 100644 --- a/src/tokenize.h +++ b/src/tokenize.h @@ -75,6 +75,8 @@ public: const Token *GetFunctionTokenByName(const char funcname[]) const; const Token *tokens() const; + std::string file(const Token *tok) const; + protected: /** Add braces to an if-block diff --git a/test/testdangerousfunctions.cpp b/test/testdangerousfunctions.cpp index c891747cf..863af4223 100644 --- a/test/testdangerousfunctions.cpp +++ b/test/testdangerousfunctions.cpp @@ -75,7 +75,7 @@ private: "{\n" " char *x = mktemp(\"/tmp/zxcv\");\n" "}\n"); - ASSERT_EQUALS(std::string("[test.cpp:3]: Found 'mktemp'. You should use 'mkstemp' instead\n"), errout.str()); + ASSERT_EQUALS(std::string("[test.cpp:3]: (style) Found 'mktemp'. You should use 'mkstemp' instead\n"), errout.str()); } void testgets() @@ -84,7 +84,7 @@ private: "{\n" " char *x = gets();\n" "}\n"); - ASSERT_EQUALS(std::string("[test.cpp:3]: Found 'gets'. You should use 'fgets' instead\n"), errout.str()); + ASSERT_EQUALS(std::string("[test.cpp:3]: (style) Found 'gets'. You should use 'fgets' instead\n"), errout.str()); } void testscanf() @@ -94,7 +94,7 @@ private: " char *x;\n" " scanf(\"%s\", x);\n" "}\n"); - ASSERT_EQUALS(std::string("[test.cpp:4]: Found 'scanf'. You should use 'fgets' instead\n"), errout.str()); + ASSERT_EQUALS(std::string("[test.cpp:4]: (style) Found 'scanf'. You should use 'fgets' instead\n"), errout.str()); } }; diff --git a/test/testsuite.cpp b/test/testsuite.cpp index f67c9cd8a..8772459e4 100644 --- a/test/testsuite.cpp +++ b/test/testsuite.cpp @@ -167,18 +167,14 @@ void TestFixture::runTests(const char cmd[]) std::cerr << errmsg.str(); } - -void TestFixture::reportErr(const std::string &errmsg) -{ - errout << errmsg << std::endl; -} - void TestFixture::reportOut(const std::string & /*outmsg*/) { // These can probably be ignored } -void TestFixture::reportXml(const std::string & /*file*/, const std::string & /*line*/, const std::string & /*id*/, const std::string & /*severity*/, const std::string & /*msg*/) +void TestFixture::reportErr(const std::list &callStack, const std::string & /*id*/, const std::string &severity, const std::string &msg) { - // These can probably be ignored + std::ostringstream text; + text << ErrorLogger::callStackToString(callStack) << ": (" << severity << ") " << msg; + errout << text.str() << std::endl; } diff --git a/test/testsuite.h b/test/testsuite.h index 33ad82386..90683f78e 100644 --- a/test/testsuite.h +++ b/test/testsuite.h @@ -42,9 +42,8 @@ protected: void assertEquals(const char *filename, int linenr, unsigned int expected, unsigned int actual); public: - virtual void reportErr(const std::string &errmsg); virtual void reportOut(const std::string &outmsg); - virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg); + virtual void reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg); void run(const std::string &str); diff --git a/tools/errmsg.cpp b/tools/errmsg.cpp index 1ea5ecadd..4b81a3f78 100644 --- a/tools/errmsg.cpp +++ b/tools/errmsg.cpp @@ -105,6 +105,12 @@ int main() err.push_back(Message("strPlusChar", Message::error, "Unusual pointer arithmetic")); err.push_back(Message("returnLocalVariable", Message::error, "Returning pointer to local array variable")); + // checkdangerousfunctions.cpp.. + err.push_back(Message("dangerousFunctionmktemp", Message::style, "Found 'mktemp'. You should use 'mkstemp' instead")); + err.push_back(Message("dangerousFunctiongets", Message::style, "Found 'gets'. You should use 'fgets' instead")); + err.push_back(Message("dangerousFunctionscanf", Message::style, "Found 'scanf'. You should use 'fgets' instead")); + + // Generate code.. std::cout << "Generate code.." << std::endl; std::ofstream fout("errorlogger.h"); @@ -137,6 +143,16 @@ int main() fout << "class Tokenizer;\n"; fout << "\n"; fout << "/**\n"; + fout << " * File name and line number.\n"; + fout << " */\n"; + fout << "class FileLocation\n"; + fout << "{\n"; + fout << "public:\n"; + fout << " std::string file;\n"; + fout << " unsigned int line;\n"; + fout << "};\n"; + fout << "\n"; + fout << "/**\n"; fout << " * This is an interface, which the class responsible of error logging\n"; fout << " * should implement.\n"; fout << " */\n"; @@ -148,15 +164,6 @@ int main() fout << " virtual ~ErrorLogger() { }\n"; fout << "\n"; fout << " /**\n"; - fout << " * Errors and warnings are directed here.\n"; - fout << " *\n"; - fout << " * @param errmsg Errors messages are normally in format\n"; - fout << " * \"[filepath:line number] Message\", e.g.\n"; - fout << " * \"[main.cpp:4] Uninitialized member variable\"\n"; - fout << " */\n"; - fout << " virtual void reportErr(const std::string &errmsg) = 0;\n"; - fout << "\n"; - fout << " /**\n"; fout << " * Information about progress is directed here.\n"; fout << " *\n"; fout << " * @param outmsg, E.g. \"Checking main.cpp...\"\n"; @@ -164,20 +171,24 @@ int main() fout << " virtual void reportOut(const std::string &outmsg) = 0;\n"; fout << "\n"; fout << " /**\n"; - fout << " * XML output of error / warning\n"; + fout << " * Output of error / warning\n"; fout << " * Todo: callstack handling\n"; fout << " *\n"; - fout << " * @param file filepath (can be \"\")\n"; - fout << " * @param line line (can be \"\")\n"; + fout << " * @param callStack File names and line numbers where the error\n"; + fout << " * was found and where it was called from. Error location is last\n"; + fout << " * element in the list\n"; fout << " * @param id error id (function name)\n"; fout << " * @param severity severity of error (always, all, style, all+style, never)\n"; fout << " * @param msg error message in plain text\n"; fout << " */\n"; - fout << " virtual void reportXml(const std::string &file, const std::string &line, const std::string &id, const std::string &severity, const std::string &msg) = 0;\n"; + fout << " virtual void reportErr(const std::list &callStack, const std::string &id, const std::string &severity, const std::string &msg) = 0;\n"; fout << "\n"; for (std::list::const_iterator it = err.begin(); it != err.end(); ++it) it->generateCode(fout); + + fout << "\n"; + fout << " static std::string callStackToString(const std::list &callStack);\n"; fout << "\n"; fout << "private:\n"; fout << " void _writemsg(const Tokenizer *tokenizer, const Token *tok, const char severity[], const std::string msg, const std::string &id);\n";