From 3ad8f98c619c1a44e987598e98da6488059d975c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 23 Jul 2010 23:12:56 +0200 Subject: [PATCH] Refactoring: Settings::addSuppression return error message and callers make sure it's reported properly. Ticket: #1839 --- lib/cppcheck.cpp | 14 ++++++++++++-- lib/preprocessor.cpp | 8 +++++++- lib/settings.cpp | 23 ++++++++++------------- lib/settings.h | 8 ++++---- test/testsettings.cpp | 4 ++-- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index bebaa87b7..72de6a01e 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -285,7 +285,12 @@ bool CppCheck::parseFromArgs(int argc, const char* const argv[]) reportOut("cppcheck: Couldn't open the file \"" + std::string(argv[i]) + "\""); return false; } - _settings.nomsg.parseFile(f); + const std::string errmsg(_settings.nomsg.parseFile(f)); + if (!errmsg.empty()) + { + reportOut(errmsg); + return false; + } } // Filter errors @@ -305,7 +310,12 @@ bool CppCheck::parseFromArgs(int argc, const char* const argv[]) reportOut("cppcheck: Couldn't open the file \"" + std::string(argv[i]) + "\""); return false; } - _settings.nofail.parseFile(f); + const std::string errmsg(_settings.nofail.parseFile(f)); + if (!errmsg.empty()) + { + reportOut(errmsg); + return false; + } } // Enables inline suppressions. diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 9e1979436..d1aefd3e2 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -238,7 +238,13 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri { // Add the suppressions. for (size_t j(0); j < suppressionIDs.size(); ++j) - settings->nomsg.addSuppression(suppressionIDs[j], filename, lineno); + { + const std::string errmsg(settings->nomsg.addSuppression(suppressionIDs[j], filename, lineno)); + if (!errmsg.empty()) + { + writeError(filename, lineno, _errorLogger, "cppcheckError", errmsg); + } + } suppressionIDs.clear(); } diff --git a/lib/settings.cpp b/lib/settings.cpp index 948abdc75..7a5a9780f 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -43,7 +43,7 @@ Settings::Settings() test_2_pass = false; } -bool Settings::Suppressions::parseFile(std::istream &istr) +std::string Settings::Suppressions::parseFile(std::istream &istr) { // Change '\r' to '\n' in the istr std::string filedata; @@ -53,8 +53,6 @@ bool Settings::Suppressions::parseFile(std::istream &istr) while (filedata.find("\r") != std::string::npos) filedata[filedata.find("\r")] = '\n'; - bool ret = true; - // Parse filedata.. std::istringstream istr2(filedata); while (std::getline(istr2, line)) @@ -76,38 +74,37 @@ bool Settings::Suppressions::parseFile(std::istream &istr) } // We could perhaps check if the id is valid and return error if it is not - ret &= addSuppression(id, file, lineNumber); + const std::string errmsg(addSuppression(id, file, lineNumber)); + if (!errmsg.empty()) + return errmsg; } - return ret; + return ""; } -bool Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line) +std::string Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line) { // Check that errorId is valid.. if (errorId.empty()) { - std::cerr << "Failed to add suppression. No id." << std::endl; - return false; + return "Failed to add suppression. No id."; } for (std::string::size_type pos = 0; pos < errorId.length(); ++pos) { if (errorId[pos] < 0 || !std::isalnum(errorId[pos])) { - std::cerr << "Failed to add suppression. Invalid id \"" << errorId << "\"" << std::endl; - return false; + return "Failed to add suppression. Invalid id \"" + errorId + "\""; } if (pos == 0 && std::isdigit(errorId[pos])) { - std::cerr << "Failed to add suppression. Invalid id \"" << errorId << "\"" << std::endl; - return false; + return "Failed to add suppression. Invalid id \"" + errorId + "\""; } } _suppressions[errorId][file].push_back(line); _suppressions[errorId][file].sort(); - return true; + return ""; } bool Settings::Suppressions::isSuppressed(const std::string &errorId, const std::string &file, unsigned int line) diff --git a/lib/settings.h b/lib/settings.h index c0acefc94..31c67e450 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -136,9 +136,9 @@ public: /** * @brief Don't show errors listed in the file. * @param istr Open file stream where errors can be read. - * @return true on success, false in syntax error is noticed. + * @return error message. empty upon success */ - bool parseFile(std::istream &istr); + std::string parseFile(std::istream &istr); /** * @brief Don't show this error. If file and/or line are optional. In which case @@ -146,9 +146,9 @@ public: * @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 on success, false in syntax error is noticed. + * @return error message. empty upon success */ - bool addSuppression(const std::string &errorId, const std::string &file = "", unsigned int line = 0); + std::string addSuppression(const std::string &errorId, const std::string &file = "", unsigned int line = 0); /** * @brief Returns true if this message should not be shown to the user. diff --git a/test/testsettings.cpp b/test/testsettings.cpp index 7abfde29c..4e3499d3a 100644 --- a/test/testsettings.cpp +++ b/test/testsettings.cpp @@ -41,14 +41,14 @@ private: { Settings::Suppressions suppressions; std::istringstream s("123"); - ASSERT_EQUALS(false, suppressions.parseFile(s)); + ASSERT_EQUALS("Failed to add suppression. Invalid id \"123\"", suppressions.parseFile(s)); } void suppressionsDosFormat() { Settings::Suppressions suppressions; std::istringstream s("abc\r\ndef\r\n"); - ASSERT_EQUALS(true, suppressions.parseFile(s)); + ASSERT_EQUALS("", suppressions.parseFile(s)); ASSERT_EQUALS(true, suppressions.isSuppressed("abc", "test.cpp", 1)); ASSERT_EQUALS(true, suppressions.isSuppressed("def", "test.cpp", 1)); }