diff --git a/AUTHORS b/AUTHORS index d225bca0e..e76ef76d6 100644 --- a/AUTHORS +++ b/AUTHORS @@ -3,6 +3,7 @@ The cppcheck team, in alphabetical order: Bill Egert Daniel Marjamäki Gianluca Scacco +Greg Hewgill Hoang Tuan Su Kimmo Varis Leandro Penz diff --git a/Makefile b/Makefile index d5fe8b8a2..a1e087771 100644 --- a/Makefile +++ b/Makefile @@ -95,6 +95,7 @@ TESTOBJ = test/options.o \ test/testsimplifytokens.o \ test/teststl.o \ test/testsuite.o \ + test/testsuppressions.o \ test/testsymboldatabase.o \ test/testthreadexecutor.o \ test/testtoken.o \ @@ -329,6 +330,9 @@ test/teststl.o: test/teststl.cpp lib/tokenize.h lib/checkstl.h lib/check.h lib/t test/testsuite.o: test/testsuite.cpp test/testsuite.h lib/errorlogger.h test/redirect.h test/options.h $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuite.o test/testsuite.cpp +test/testsuppressions.o: test/testsuppressions.cpp test/testsuite.h lib/cppcheck.h lib/settings.h lib/errorlogger.h cli/threadexecutor.h + $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuppressions.o test/testsuppressions.cpp + test/testsymboldatabase.o: test/testsymboldatabase.cpp test/testsuite.h lib/errorlogger.h test/redirect.h test/testutils.h lib/settings.h lib/tokenize.h lib/token.h lib/symboldatabase.h $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsymboldatabase.o test/testsymboldatabase.cpp diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index c101b6592..491f48caa 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -123,7 +123,7 @@ bool CppCheckExecutor::parseFromArgs(CppCheck *cppcheck, int argc, const char* c int CppCheckExecutor::check(int argc, const char* const argv[]) { - CppCheck cppCheck(*this); + CppCheck cppCheck(*this, true); if (!parseFromArgs(&cppCheck, argc, argv)) { return EXIT_FAILURE; @@ -152,11 +152,13 @@ int CppCheckExecutor::check(int argc, const char* const argv[]) { // Multiple processes const std::vector &filenames = cppCheck.filenames(); - Settings settings = cppCheck.settings(); + Settings &settings = cppCheck.settings(); ThreadExecutor executor(filenames, settings, *this); returnValue = executor.check(); } + reportUnmatchedSuppressions(cppCheck.settings().nomsg.getUnmatchedGlobalSuppressions()); + if (_settings._xml) { reportErr(ErrorLogger::ErrorMessage::getXMLFooter(_settings._xml_version)); diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index c4119876d..7c33d2adb 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -31,7 +31,7 @@ #include #endif -ThreadExecutor::ThreadExecutor(const std::vector &filenames, const Settings &settings, ErrorLogger &errorLogger) +ThreadExecutor::ThreadExecutor(const std::vector &filenames, Settings &settings, ErrorLogger &errorLogger) : _filenames(filenames), _settings(settings), _errorLogger(errorLogger), _fileCount(0) { #if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) @@ -95,12 +95,23 @@ int ThreadExecutor::handleRead(unsigned int &result) ErrorLogger::ErrorMessage msg; msg.deserialize(buf); - // Alert only about unique errors - std::string errmsg = msg.toString(_settings._verbose); - if (std::find(_errorList.begin(), _errorList.end(), errmsg) == _errorList.end()) + std::string file; + unsigned int line(0); + if (!msg._callStack.empty()) { - _errorList.push_back(errmsg); - _errorLogger.reportErr(msg); + file = msg._callStack.back().getfile(false); + line = msg._callStack.back().line; + } + + if (!_settings.nomsg.isSuppressed(msg._id, file, line)) + { + // Alert only about unique errors + std::string errmsg = msg.toString(_settings._verbose); + if (std::find(_errorList.begin(), _errorList.end(), errmsg) == _errorList.end()) + { + _errorList.push_back(errmsg); + _errorLogger.reportErr(msg); + } } } else if (type == '3') @@ -158,7 +169,7 @@ unsigned int ThreadExecutor::check() } else if (pid == 0) { - CppCheck fileChecker(*this); + CppCheck fileChecker(*this, false); fileChecker.settings(_settings); if (_fileContents.size() > 0 && _fileContents.find(_filenames[i]) != _fileContents.end()) diff --git a/cli/threadexecutor.h b/cli/threadexecutor.h index 1fb0af877..309e110ee 100644 --- a/cli/threadexecutor.h +++ b/cli/threadexecutor.h @@ -35,7 +35,7 @@ class ThreadExecutor : public ErrorLogger { public: - ThreadExecutor(const std::vector &filenames, const Settings &settings, ErrorLogger &_errorLogger); + ThreadExecutor(const std::vector &filenames, Settings &settings, ErrorLogger &_errorLogger); virtual ~ThreadExecutor(); unsigned int check(); virtual void reportOut(const std::string &outmsg); @@ -52,7 +52,7 @@ public: private: const std::vector &_filenames; - const Settings &_settings; + Settings &_settings; ErrorLogger &_errorLogger; unsigned int _fileCount; diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 494feac58..4febd096b 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -37,8 +37,8 @@ static TimerResults S_timerResults; -CppCheck::CppCheck(ErrorLogger &errorLogger) - : _errorLogger(errorLogger) +CppCheck::CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions) + : _useGlobalSuppressions(useGlobalSuppressions), _errorLogger(errorLogger) { exitcode = 0; } @@ -197,6 +197,8 @@ unsigned int CppCheck::check() _errorLogger.reportOut("Bailing out from checking " + fixedpath + ": " + e.what()); } + reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions()); + _errorLogger.reportStatus(c + 1, (unsigned int)_filenames.size()); } @@ -385,7 +387,7 @@ void CppCheck::checkFile(const std::string &code, const char FileName[]) #endif } -Settings CppCheck::settings() const +Settings &CppCheck::settings() { return _settings; } @@ -408,8 +410,16 @@ void CppCheck::reportErr(const ErrorLogger::ErrorMessage &msg) line = msg._callStack.back().line; } - if (_settings.nomsg.isSuppressed(msg._id, file, line)) - return; + if (_useGlobalSuppressions) + { + if (_settings.nomsg.isSuppressed(msg._id, file, line)) + return; + } + else + { + if (_settings.nomsg.isSuppressedLocal(msg._id, file, line)) + return; + } if (!_settings.nofail.isSuppressed(msg._id, file, line)) exitcode = 1; diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 9f00c86b2..3e02a4e3e 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -43,7 +43,7 @@ public: /** * @brief Constructor. */ - CppCheck(ErrorLogger &errorLogger); + CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions); /** * @brief Destructor. @@ -66,10 +66,10 @@ public: void settings(const Settings &settings); /** - * @brief Get copy of current settings. - * @return a copy of current settings + * @brief Get reference to current settings. + * @return a reference to current settings */ - Settings settings() const; + Settings &settings(); /** * @brief Add new file to be checked. @@ -147,6 +147,7 @@ private: std::list _errorList; std::ostringstream _errout; Settings _settings; + bool _useGlobalSuppressions; std::vector _filenames; void reportProgress(const std::string &filename, const char stage[], const unsigned int value); diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 387603d8f..c96c12854 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -291,11 +291,26 @@ std::string ErrorLogger::ErrorMessage::toString(bool verbose, const std::string } } +void ErrorLogger::reportUnmatchedSuppressions(const std::list &unmatched) +{ + for (std::list::const_iterator i = unmatched.begin(); i != unmatched.end(); ++i) + { + std::list callStack; + callStack.push_back(ErrorLogger::ErrorMessage::FileLocation(i->file, i->line)); + reportErr(ErrorLogger::ErrorMessage(callStack, Severity::information, "Unmatched suppression: " + i->id, "unmatchedSuppression")); + } +} + 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).getfile() << ":" << (*tok).line << "]"; + { + ostr << (tok == callStack.begin() ? "" : " -> ") << "[" << (*tok).getfile(); + if ((*tok).line != 0) + ostr << ":" << (*tok).line; + ostr << "]"; + } return ostr.str(); } diff --git a/lib/errorlogger.h b/lib/errorlogger.h index da5b7835f..410bbf4c4 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -23,6 +23,8 @@ #include #include +#include "settings.h" + class Token; class Tokenizer; @@ -108,6 +110,11 @@ public: line = 0; } + FileLocation(const std::string &file, int aline) + : line(aline), _file(file) + { + } + /** * Return the filename. * @param convert If true convert path to native separators. @@ -230,6 +237,12 @@ public: (void)value; } + /** + * Report list of unmatched suppressions + * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions) + */ + void reportUnmatchedSuppressions(const std::list &unmatched); + static std::string callStackToString(const std::list &callStack); }; diff --git a/lib/settings.cpp b/lib/settings.cpp index dbd4f37c7..b523c1903 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -70,46 +70,7 @@ std::string Settings::Suppressions::parseFile(std::istream &istr) if (line.length() >= 2 && line[0] == '/' && line[1] == '/') continue; - std::istringstream lineStream(line); - std::string id; - std::string file; - unsigned int lineNumber = 0; - if (std::getline(lineStream, id, ':')) - { - if (std::getline(lineStream, file)) - { - // If there is not a dot after the last colon in "file" then - // the colon is a separator and the contents after the colon - // is a line number.. - - // Get position of last colon - const std::string::size_type pos = file.rfind(":"); - - // if a colon is found and there is no dot after it.. - if (pos != std::string::npos && - file.find(".", pos) == std::string::npos) - { - // Try to parse out the line number - try - { - std::istringstream istr1(file.substr(pos+1)); - istr1 >> lineNumber; - } - catch (...) - { - lineNumber = 0; - } - - if (lineNumber > 0) - { - file.erase(pos); - } - } - } - } - - // We could perhaps check if the id is valid and return error if it is not - const std::string errmsg(addSuppression(id, file, lineNumber)); + const std::string errmsg(addSuppressionLine(line)); if (!errmsg.empty()) return errmsg; } @@ -117,6 +78,54 @@ std::string Settings::Suppressions::parseFile(std::istream &istr) return ""; } +std::string Settings::Suppressions::addSuppressionLine(const std::string &line) +{ + std::istringstream lineStream(line); + std::string id; + std::string file; + unsigned int lineNumber = 0; + if (std::getline(lineStream, id, ':')) + { + if (std::getline(lineStream, file)) + { + // If there is not a dot after the last colon in "file" then + // the colon is a separator and the contents after the colon + // is a line number.. + + // Get position of last colon + const std::string::size_type pos = file.rfind(":"); + + // if a colon is found and there is no dot after it.. + if (pos != std::string::npos && + file.find(".", pos) == std::string::npos) + { + // Try to parse out the line number + try + { + std::istringstream istr1(file.substr(pos+1)); + istr1 >> lineNumber; + } + catch (...) + { + lineNumber = 0; + } + + if (lineNumber > 0) + { + file.erase(pos); + } + } + } + } + + // We could perhaps check if the id is valid and return error if it is not + const std::string errmsg(addSuppression(id, file, lineNumber)); + if (!errmsg.empty()) + return errmsg; + + return ""; +} + bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, const std::string &name) { const char *p = pattern.c_str(); @@ -205,47 +214,64 @@ std::string Settings::Suppressions::FileMatcher::addFile(const std::string &name } } } - _globs[name].insert(line); + _globs[name][line] = false; + } + else if (name.empty()) + { + _globs["*"][0U] = false; } else { - _files[name].insert(line); + _files[name][line] = false; } return ""; } bool Settings::Suppressions::FileMatcher::isSuppressed(const std::string &file, unsigned int line) { - // Check are all errors of this type filtered out - if (_files.find("") != _files.end()) + if (isSuppressedLocal(file, line)) return true; - std::set lineset; - - std::map >::const_iterator f = _files.find(file); - if (f != _files.end()) - { - lineset.insert(f->second.begin(), f->second.end()); - } - for (std::map >::iterator g = _globs.begin(); g != _globs.end(); ++g) + for (std::map >::iterator g = _globs.begin(); g != _globs.end(); ++g) { if (match(g->first, file)) { - lineset.insert(g->second.begin(), g->second.end()); + if (g->second.find(0U) != g->second.end()) + { + g->second[0U] = true; + return true; + } + std::map::iterator l = g->second.find(line); + if (l != g->second.end()) + { + l->second = true; + return true; + } } } - if (lineset.empty()) - return false; + return false; +} - // Check should all errors in this file be filtered out - if (lineset.find(0U) != lineset.end()) - return true; +bool Settings::Suppressions::FileMatcher::isSuppressedLocal(const std::string &file, unsigned int line) +{ + std::map >::iterator f = _files.find(file); + if (f != _files.end()) + { + if (f->second.find(0U) != f->second.end()) + { + f->second[0U] = true; + return true; + } + std::map::iterator l = f->second.find(line); + if (l != f->second.end()) + { + l->second = true; + return true; + } + } - if (lineset.find(line) == lineset.end()) - return false; - - return true; + return false; } std::string Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line) @@ -278,6 +304,52 @@ bool Settings::Suppressions::isSuppressed(const std::string &errorId, const std: return _suppressions[errorId].isSuppressed(file, line); } +bool Settings::Suppressions::isSuppressedLocal(const std::string &errorId, const std::string &file, unsigned int line) +{ + if (_suppressions.find(errorId) == _suppressions.end()) + return false; + + return _suppressions[errorId].isSuppressedLocal(file, line); +} + +std::list Settings::Suppressions::getUnmatchedLocalSuppressions() const +{ + std::list r; + for (std::map::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) + { + for (std::map >::const_iterator f = i->second._files.begin(); f != i->second._files.end(); ++f) + { + for (std::map::const_iterator l = f->second.begin(); l != f->second.end(); ++l) + { + if (!l->second) + { + r.push_back(SuppressionEntry(i->first, f->first, l->first)); + } + } + } + } + return r; +} + +std::list Settings::Suppressions::getUnmatchedGlobalSuppressions() const +{ + std::list r; + for (std::map::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) + { + for (std::map >::const_iterator g = i->second._globs.begin(); g != i->second._globs.end(); ++g) + { + for (std::map::const_iterator l = g->second.begin(); l != g->second.end(); ++l) + { + if (!l->second) + { + r.push_back(SuppressionEntry(i->first, g->first, l->first)); + } + } + } + } + return r; +} + std::string Settings::addEnabled(const std::string &str) { // Enable parameters may be comma separated... diff --git a/lib/settings.h b/lib/settings.h index d2d9a217d..b65d741c4 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -138,11 +138,12 @@ public: private: class FileMatcher { + friend class Suppressions; private: - /** @brief List of filenames suppressed. */ - std::map > _files; - /** @brief List of globs suppressed. */ - std::map > _globs; + /** @brief List of filenames suppressed, bool flag indicates whether suppression matched. */ + std::map > _files; + /** @brief List of globs suppressed, bool flag indicates whether suppression matched. */ + std::map > _globs; /** * @brief Match a name against a glob pattern. @@ -168,6 +169,14 @@ public: * @return true if this filename/line matches */ bool isSuppressed(const std::string &file, unsigned int line); + + /** + * @brief Returns true if the file name matches a previously added file (only, not glob pattern). + * @param name File name to check + * @param line Line number + * @return true if this filename/line matches + */ + bool isSuppressedLocal(const std::string &file, unsigned int line); }; /** @brief List of error which the user doesn't want to see. */ @@ -180,6 +189,13 @@ public: */ std::string parseFile(std::istream &istr); + /** + * @brief Don't show the given error. + * @param str Description of error to suppress (in id:file:line format). + * @return error message. empty upon success + */ + std::string addSuppressionLine(const std::string &line); + /** * @brief Don't show this error. If file and/or line are optional. In which case * the errorId alone is used for filtering. @@ -198,6 +214,38 @@ public: * @return true if this error is suppressed. */ bool isSuppressed(const std::string &errorId, const std::string &file, unsigned int line); + + /** + * @brief Returns true if this message should not be shown to the user (explicit files only, not glob patterns). + * @param errorId the id for the error, e.g. "arrayIndexOutOfBounds" + * @param file File name with the path, e.g. "src/main.cpp" + * @param line number, e.g. "123" + * @return true if this error is suppressed. + */ + bool isSuppressedLocal(const std::string &errorId, const std::string &file, unsigned int line); + + struct SuppressionEntry + { + SuppressionEntry(const std::string &aid, const std::string &afile, const unsigned int &aline) + : id(aid), file(afile), line(aline) + { } + + std::string id; + std::string file; + unsigned int line; + }; + + /** + * @brief Returns list of unmatched local (per-file) suppressions. + * @return list of unmatched suppressions + */ + std::list getUnmatchedLocalSuppressions() const; + + /** + * @brief Returns list of unmatched global (glob pattern) suppressions. + * @return list of unmatched suppressions + */ + std::list getUnmatchedGlobalSuppressions() const; }; /** @brief suppress message (--suppressions) */ diff --git a/test/testcppcheck.cpp b/test/testcppcheck.cpp index d9b7ee5f0..f4fbb3f13 100644 --- a/test/testcppcheck.cpp +++ b/test/testcppcheck.cpp @@ -86,7 +86,7 @@ private: void getErrorMessages() { ErrorLogger2 errorLogger; - CppCheck cppCheck(errorLogger); + CppCheck cppCheck(errorLogger, true); cppCheck.getErrorMessages(); ASSERT(!errorLogger.id.empty()); diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp new file mode 100644 index 000000000..b8adaad47 --- /dev/null +++ b/test/testsuppressions.cpp @@ -0,0 +1,172 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "cppcheck.h" +#include "settings.h" +#include "testsuite.h" +#include "threadexecutor.h" + +#include + +extern std::ostringstream errout; + +class TestSuppressions : public TestFixture +{ +public: + TestSuppressions() : TestFixture("TestSuppressions") + { } + +private: + + void run() + { + TEST_CASE(suppressionsSettings); + } + + // Check the suppression + void checkSuppression(const char code[], const std::string &suppression = "") + { + // Clear the error log + errout.str(""); + + Settings settings; + settings._inlineSuppressions = true; + if (!suppression.empty()) + settings.nomsg.addSuppressionLine(suppression); + + CppCheck cppCheck(*this, true); + cppCheck.settings(settings); + cppCheck.addFile("test.cpp", code); + cppCheck.check(); + + reportUnmatchedSuppressions(cppCheck.settings().nomsg.getUnmatchedGlobalSuppressions()); + } + + void checkSuppressionThreads(const char code[], const std::string &suppression = "") + { + errout.str(""); + output.str(""); + if (!ThreadExecutor::isEnabled()) + { + // Skip this check on systems which don't use this feature + return; + } + + std::vector filenames; + filenames.push_back("test.cpp"); + + Settings settings; + settings._jobs = 1; + settings._inlineSuppressions = true; + if (!suppression.empty()) + settings.nomsg.addSuppressionLine(suppression); + ThreadExecutor executor(filenames, settings, *this); + for (unsigned int i = 0; i < filenames.size(); ++i) + executor.addFileContent(filenames[i], code); + + executor.check(); + + reportUnmatchedSuppressions(settings.nomsg.getUnmatchedGlobalSuppressions()); + } + + void runChecks(void (TestSuppressions::*check)(const char[], const std::string &)) + { + // check to make sure the appropriate error is present + (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + ""); + ASSERT_EQUALS("[test.cpp:3]: (error) Uninitialized variable: a\n", errout.str()); + + // suppress uninitvar globally + (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "uninitvar"); + ASSERT_EQUALS("", errout.str()); + + // suppress uninitvar globally, without error present + (this->*check)("void f() {\n" + " int a;\n" + " b++;\n" + "}\n", + "uninitvar"); + ASSERT_EQUALS("[*]: (information) Unmatched suppression: uninitvar\n", errout.str()); + + // suppress uninitvar for this file only + (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "uninitvar:test.cpp"); + ASSERT_EQUALS("", errout.str()); + + // suppress uninitvar for this file only, without error present + (this->*check)("void f() {\n" + " int a;\n" + " b++;\n" + "}\n", + "uninitvar:test.cpp"); + ASSERT_EQUALS("[test.cpp]: (information) Unmatched suppression: uninitvar\n", errout.str()); + + // suppress uninitvar for this file and line + (this->*check)("void f() {\n" + " int a;\n" + " a++;\n" + "}\n", + "uninitvar:test.cpp:3"); + ASSERT_EQUALS("", errout.str()); + + // suppress uninitvar for this file and line, without error present + (this->*check)("void f() {\n" + " int a;\n" + " b++;\n" + "}\n", + "uninitvar:test.cpp:3"); + ASSERT_EQUALS("[test.cpp:3]: (information) Unmatched suppression: uninitvar\n", errout.str()); + + // suppress uninitvar inline + (this->*check)("void f() {\n" + " int a;\n" + " // cppcheck-suppress uninitvar\n" + " a++;\n" + "}\n", + ""); + ASSERT_EQUALS("", errout.str()); + + // suppress uninitvar inline, without error present + (this->*check)("void f() {\n" + " int a;\n" + " // cppcheck-suppress uninitvar\n" + " b++;\n" + "}\n", + ""); + ASSERT_EQUALS("[test.cpp:4]: (information) Unmatched suppression: uninitvar\n", errout.str()); + } + + void suppressionsSettings() + { + runChecks(&TestSuppressions::checkSuppression); + runChecks(&TestSuppressions::checkSuppressionThreads); + } + +}; + +REGISTER_TEST(TestSuppressions)