From 38986302e91fafacb6e9599e1e220c15c035d933 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Tue, 8 Feb 2011 21:44:59 +1300 Subject: [PATCH 1/5] failing test for suppression glob --- test/testsettings.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/testsettings.cpp b/test/testsettings.cpp index 28e242543..88a5eeda5 100644 --- a/test/testsettings.cpp +++ b/test/testsettings.cpp @@ -36,6 +36,7 @@ private: TEST_CASE(suppressionsBadId1); TEST_CASE(suppressionsDosFormat); // Ticket #1836 TEST_CASE(suppressionsFileNameWithColon); // Ticket #1919 - filename includes colon + TEST_CASE(suppressionsGlob); } void suppressionsBadId1() @@ -63,6 +64,15 @@ private: ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "c:\\bar.cpp", 10)); ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "c:\\bar.cpp", 12)); } + + void suppressionsGlob() + { + Settings::Suppressions suppressions; + std::istringstream s("error:x*.cpp\n"); + ASSERT_EQUALS("", suppressions.parseFile(s)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 1)); + ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "abc.cpp", 1)); + } }; REGISTER_TEST(TestSettings) From a9f28798892811dee32dfaf0b1ab6bcd008b9dd6 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Tue, 8 Feb 2011 21:45:17 +1300 Subject: [PATCH 2/5] factor out file matching into own class --- lib/settings.cpp | 46 +++++++++++++++++++++++++++------------------- lib/settings.h | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/lib/settings.cpp b/lib/settings.cpp index e0fc89257..007edcac6 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -116,6 +116,31 @@ std::string Settings::Suppressions::parseFile(std::istream &istr) return ""; } +std::string Settings::Suppressions::FileMatcher::addFile(const std::string &name, unsigned int line) +{ + _files[name].insert(line); + 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()) + return true; + + if (_files.find(file) == _files.end()) + return false; + + // Check should all errors in this file be filtered out + if (std::find(_files[file].begin(), _files[file].end(), 0U) != _files[file].end()) + return true; + + if (std::find(_files[file].begin(), _files[file].end(), line) == _files[file].end()) + return false; + + return true; +} + std::string Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line) { // Check that errorId is valid.. @@ -135,10 +160,7 @@ std::string Settings::Suppressions::addSuppression(const std::string &errorId, c } } - _suppressions[errorId][file].push_back(line); - _suppressions[errorId][file].sort(); - - return ""; + return _suppressions[errorId].addFile(file, line); } bool Settings::Suppressions::isSuppressed(const std::string &errorId, const std::string &file, unsigned int line) @@ -146,21 +168,7 @@ bool Settings::Suppressions::isSuppressed(const std::string &errorId, const std: if (_suppressions.find(errorId) == _suppressions.end()) return false; - // Check are all errors of this type filtered out - if (_suppressions[errorId].find("") != _suppressions[errorId].end()) - return true; - - if (_suppressions[errorId].find(file) == _suppressions[errorId].end()) - return false; - - // Check should all errors in this file be filtered out - if (std::find(_suppressions[errorId][file].begin(), _suppressions[errorId][file].end(), 0) != _suppressions[errorId][file].end()) - return true; - - if (std::find(_suppressions[errorId][file].begin(), _suppressions[errorId][file].end(), line) == _suppressions[errorId][file].end()) - return false; - - return true; + return _suppressions[errorId].isSuppressed(file, line); } std::string Settings::addEnabled(const std::string &str) diff --git a/lib/settings.h b/lib/settings.h index c6bec4336..d2d9a217d 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -23,6 +23,7 @@ #include #include #include +#include /// @addtogroup Core /// @{ @@ -135,8 +136,42 @@ public: class Suppressions { private: + class FileMatcher + { + private: + /** @brief List of filenames suppressed. */ + std::map > _files; + /** @brief List of globs suppressed. */ + std::map > _globs; + + /** + * @brief Match a name against a glob pattern. + * @param pattern The glob pattern to match. + * @param name The filename to match against the glob pattern. + * @return match success + */ + static bool match(const std::string &pattern, const std::string &name); + + public: + /** + * @brief Add a file or glob (and line number). + * @param name File name or glob pattern + * @param line Line number + * @return error message. empty upon success + */ + std::string addFile(const std::string &name, unsigned int line); + + /** + * @brief Returns true if the file name matches a previously added file or glob pattern. + * @param name File name to check + * @param line Line number + * @return true if this filename/line matches + */ + bool isSuppressed(const std::string &file, unsigned int line); + }; + /** @brief List of error which the user doesn't want to see. */ - std::map > > _suppressions; + std::map _suppressions; public: /** * @brief Don't show errors listed in the file. From 7a219b1fb8354634c48820f3ea699ad9a2f6e473 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Wed, 9 Feb 2011 00:13:50 +1300 Subject: [PATCH 3/5] support wildcard characters * and ? in suppression list --- lib/settings.cpp | 92 +++++++++++++++++++++++++++++++++++++++++-- man/cppcheck.1.xml | 3 +- man/manual.docbook | 4 ++ test/testsettings.cpp | 36 ++++++++++++++--- 4 files changed, 125 insertions(+), 10 deletions(-) diff --git a/lib/settings.cpp b/lib/settings.cpp index 007edcac6..bf9f6a58e 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -116,9 +116,81 @@ std::string Settings::Suppressions::parseFile(std::istream &istr) return ""; } +bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, const std::string &name) +{ + const char *p = pattern.c_str(); + const char *n = name.c_str(); + std::list > backtrack; + + for (;;) { + bool matching = true; + while (*p != '\0' && matching) { + switch (*p) { + case '*': + // Step forward until we match the next character after * + while (*n != '\0' && *n != p[1]) { + n++; + } + if (*n != '\0') { + // If this isn't the last possibility, save it for later + backtrack.push_back(std::make_pair(p, n)); + } + break; + case '?': + // Any character matches unless we're at the end of the name + if (*n != '\0') { + n++; + } else { + matching = false; + } + break; + default: + // Non-wildcard characters match literally + if (*n == *p) { + n++; + } else { + matching = false; + } + break; + } + p++; + } + + // If we haven't failed matching and we've reached the end of the name, then success + if (matching && *n == '\0') { + return true; + } + + // If there are no other paths to tray, then fail + if (backtrack.empty()) { + return false; + } + + // Restore pointers from backtrack stack + p = backtrack.back().first; + n = backtrack.back().second; + backtrack.pop_back(); + + // Advance name pointer by one because the current position didn't work + n++; + } +} + std::string Settings::Suppressions::FileMatcher::addFile(const std::string &name, unsigned int line) { - _files[name].insert(line); + if (name.find_first_of("*?") != std::string::npos) { + for (std::string::const_iterator i = name.begin(); i != name.end(); ++i) { + if (*i == '*') { + std::string::const_iterator j = i + 1; + if (j != name.end() && (*j == '*' || *j == '?')) { + return "Failed to add suppression. Syntax error in glob."; + } + } + } + _globs[name].insert(line); + } else { + _files[name].insert(line); + } return ""; } @@ -128,14 +200,26 @@ bool Settings::Suppressions::FileMatcher::isSuppressed(const std::string &file, if (_files.find("") != _files.end()) return true; - if (_files.find(file) == _files.end()) + 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) { + if (match(g->first, file)) { + lineset.insert(g->second.begin(), g->second.end()); + } + } + + if (lineset.empty()) return false; // Check should all errors in this file be filtered out - if (std::find(_files[file].begin(), _files[file].end(), 0U) != _files[file].end()) + if (lineset.find(0U) != lineset.end()) return true; - if (std::find(_files[file].begin(), _files[file].end(), line) == _files[file].end()) + if (lineset.find(line) == lineset.end()) return false; return true; diff --git a/man/cppcheck.1.xml b/man/cppcheck.1.xml index 3344d02b3..9332a09ae 100644 --- a/man/cppcheck.1.xml +++ b/man/cppcheck.1.xml @@ -307,7 +307,8 @@ Directory name is matched to all parts of the path. Suppress warnings listed in the file. Filename and line are optional. The format of the single line in file is: [error id]:[filename]:[line]. - You can use --template or --xml to see the error id. + You can use --template or --xml to see the error id. + The filename may contain the wildcard characters * or ?. diff --git a/man/manual.docbook b/man/manual.docbook index 02a25bf73..07285513a 100644 --- a/man/manual.docbook +++ b/man/manual.docbook @@ -385,6 +385,10 @@ gui/test.cpp,16,error,mismatchAllocDealloc,Mismatching allocation and deallocati line flag. Copy and paste the id string from the XML output. + The filename may include the wildcard characters + * or ?, which match any sequence of characters or any single character + respectively. + Here is an example: memleak:file1.cpp diff --git a/test/testsettings.cpp b/test/testsettings.cpp index 88a5eeda5..078324be2 100644 --- a/test/testsettings.cpp +++ b/test/testsettings.cpp @@ -67,11 +67,37 @@ private: void suppressionsGlob() { - Settings::Suppressions suppressions; - std::istringstream s("error:x*.cpp\n"); - ASSERT_EQUALS("", suppressions.parseFile(s)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 1)); - ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "abc.cpp", 1)); + // Check for syntax errors in glob + { + Settings::Suppressions suppressions; + std::istringstream s("errorid:**.cpp\n"); + ASSERT_EQUALS("Failed to add suppression. Syntax error in glob.", suppressions.parseFile(s)); + } + + // Check that globbing works + { + Settings::Suppressions suppressions; + std::istringstream s("errorid:x*.cpp\nerrorid:y?.cpp\nerrorid:test.c*"); + ASSERT_EQUALS("", suppressions.parseFile(s)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp.cpp", 1)); + ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "abc.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "ya.cpp", 1)); + ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "y.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "test.c", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "test.cpp", 1)); + } + + // Check that both a filename match and a glob match apply + { + Settings::Suppressions suppressions; + std::istringstream s("errorid:x*.cpp\nerrorid:xyz.cpp:1\nerrorid:a*.cpp:1\nerrorid:abc.cpp:2"); + ASSERT_EQUALS("", suppressions.parseFile(s)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 2)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "abc.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "abc.cpp", 2)); + } } }; From 421b32efb46dd7d99f0e7d4ad410d61dde22eb71 Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Sun, 13 Feb 2011 09:55:45 +1300 Subject: [PATCH 4/5] use std::stack instead of std::list where appropriate --- lib/settings.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/settings.cpp b/lib/settings.cpp index bf9f6a58e..88cb556ab 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -25,6 +25,7 @@ #include #include // std::isdigit, std::isalnum, etc #include +#include Settings::Settings() { @@ -120,7 +121,7 @@ bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, cons { const char *p = pattern.c_str(); const char *n = name.c_str(); - std::list > backtrack; + std::stack > backtrack; for (;;) { bool matching = true; @@ -133,7 +134,7 @@ bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, cons } if (*n != '\0') { // If this isn't the last possibility, save it for later - backtrack.push_back(std::make_pair(p, n)); + backtrack.push(std::make_pair(p, n)); } break; case '?': @@ -167,9 +168,9 @@ bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, cons } // Restore pointers from backtrack stack - p = backtrack.back().first; - n = backtrack.back().second; - backtrack.pop_back(); + p = backtrack.top().first; + n = backtrack.top().second; + backtrack.pop(); // Advance name pointer by one because the current position didn't work n++; From 1418c12261b4fa70643c32c02f90150236ee32cd Mon Sep 17 00:00:00 2001 From: Greg Hewgill Date: Sun, 13 Feb 2011 10:01:32 +1300 Subject: [PATCH 5/5] astyle formatting --- lib/settings.cpp | 100 +++++++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 39 deletions(-) diff --git a/lib/settings.cpp b/lib/settings.cpp index 88cb556ab..dbd4f37c7 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -123,47 +123,60 @@ bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, cons const char *n = name.c_str(); std::stack > backtrack; - for (;;) { + for (;;) + { bool matching = true; - while (*p != '\0' && matching) { - switch (*p) { - case '*': - // Step forward until we match the next character after * - while (*n != '\0' && *n != p[1]) { - n++; - } - if (*n != '\0') { - // If this isn't the last possibility, save it for later - backtrack.push(std::make_pair(p, n)); - } - break; - case '?': - // Any character matches unless we're at the end of the name - if (*n != '\0') { - n++; - } else { - matching = false; - } - break; - default: - // Non-wildcard characters match literally - if (*n == *p) { - n++; - } else { - matching = false; - } - break; + while (*p != '\0' && matching) + { + switch (*p) + { + case '*': + // Step forward until we match the next character after * + while (*n != '\0' && *n != p[1]) + { + n++; + } + if (*n != '\0') + { + // If this isn't the last possibility, save it for later + backtrack.push(std::make_pair(p, n)); + } + break; + case '?': + // Any character matches unless we're at the end of the name + if (*n != '\0') + { + n++; + } + else + { + matching = false; + } + break; + default: + // Non-wildcard characters match literally + if (*n == *p) + { + n++; + } + else + { + matching = false; + } + break; } p++; } // If we haven't failed matching and we've reached the end of the name, then success - if (matching && *n == '\0') { + if (matching && *n == '\0') + { return true; } // If there are no other paths to tray, then fail - if (backtrack.empty()) { + if (backtrack.empty()) + { return false; } @@ -179,17 +192,23 @@ bool Settings::Suppressions::FileMatcher::match(const std::string &pattern, cons std::string Settings::Suppressions::FileMatcher::addFile(const std::string &name, unsigned int line) { - if (name.find_first_of("*?") != std::string::npos) { - for (std::string::const_iterator i = name.begin(); i != name.end(); ++i) { - if (*i == '*') { + if (name.find_first_of("*?") != std::string::npos) + { + for (std::string::const_iterator i = name.begin(); i != name.end(); ++i) + { + if (*i == '*') + { std::string::const_iterator j = i + 1; - if (j != name.end() && (*j == '*' || *j == '?')) { + if (j != name.end() && (*j == '*' || *j == '?')) + { return "Failed to add suppression. Syntax error in glob."; } } } _globs[name].insert(line); - } else { + } + else + { _files[name].insert(line); } return ""; @@ -204,11 +223,14 @@ bool Settings::Suppressions::FileMatcher::isSuppressed(const std::string &file, std::set lineset; std::map >::const_iterator f = _files.find(file); - if (f != _files.end()) { + if (f != _files.end()) + { lineset.insert(f->second.begin(), f->second.end()); } - for (std::map >::iterator g = _globs.begin(); g != _globs.end(); ++g) { - if (match(g->first, file)) { + for (std::map >::iterator g = _globs.begin(); g != _globs.end(); ++g) + { + if (match(g->first, file)) + { lineset.insert(g->second.begin(), g->second.end()); } }