From fdded6268e9623a84823449e746255868d34894c Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Apr 2026 09:48:10 +0200 Subject: [PATCH 1/3] TestSuppressions: added more `parseLine()` tests --- test/testsuppressions.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index eaa524e3640..b1f919b4b2a 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -146,13 +146,34 @@ class TestSuppressions : public TestFixture { void parseLine() const { + ASSERT_EQUALS("bad", SuppressionList::parseLine("bad").toString()); + ASSERT_EQUALS("bad:test.c", SuppressionList::parseLine("bad:test.c").toString()); ASSERT_EQUALS("bad:test.c:1", SuppressionList::parseLine("bad:test.c:1").toString()); // symbol + ASSERT_EQUALS("bad\nsymbol=x", SuppressionList::parseLine("bad\nsymbol=x").toString()); + ASSERT_EQUALS("bad:test.c\nsymbol=x", SuppressionList::parseLine("bad:test.c\nsymbol=x").toString()); ASSERT_EQUALS("bad:test.c:1\nsymbol=x", SuppressionList::parseLine("bad:test.c:1\nsymbol=x").toString()); + // empty symbol + ASSERT_EQUALS("bad", SuppressionList::parseLine("bad\nsymbol=").toString()); + ASSERT_EQUALS("bad:test.c", SuppressionList::parseLine("bad:test.c\nsymbol=").toString()); + ASSERT_EQUALS("bad:test.c:1", SuppressionList::parseLine("bad:test.c:1\nsymbol=").toString()); + // polyspace + ASSERT_EQUALS("bad\npolyspace=1", SuppressionList::parseLine("bad\npolyspace=1").toString()); + ASSERT_EQUALS("bad:test.c\npolyspace=1", SuppressionList::parseLine("bad:test.c\npolyspace=1").toString()); ASSERT_EQUALS("bad:test.c:1\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\npolyspace=1").toString()); + + // symbol + polyspace + ASSERT_EQUALS("bad\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad\nsymbol=x\npolyspace=1").toString()); + ASSERT_EQUALS("bad:test.c\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c\nsymbol=x\npolyspace=1").toString()); + ASSERT_EQUALS("bad:test.c:1\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\nsymbol=x\npolyspace=1").toString()); + + // polyspace + symbol + ASSERT_EQUALS("bad\npolyspace=1\nsymbol=x", SuppressionList::parseLine("bad\npolyspace=1\nsymbol=x").toString()); + ASSERT_EQUALS("bad:test.c\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c\npolyspace=1\nsymbol=x").toString()); + ASSERT_EQUALS("bad:test.c:1\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\npolyspace=1\nsymbol=x").toString()); } void suppressionsBadId1() const { From 91f6243453cf2294dd6c549367363687ea48b1bf Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Apr 2026 09:48:51 +0200 Subject: [PATCH 2/3] fixed #14638 - report errors for invalid suppression lines --- lib/suppressions.cpp | 69 +++++++++++++++++++++++---------------- lib/suppressions.h | 3 +- test/testsuppressions.cpp | 31 +++++++++++++++++- 3 files changed, 73 insertions(+), 30 deletions(-) diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index 241ec3d1f9f..577af9a9616 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -207,9 +207,8 @@ std::vector SuppressionList::parseMultiSuppressCom return suppressions; } -SuppressionList::Suppression SuppressionList::parseLine(const std::string &line) +SuppressionList::Suppression SuppressionList::parseLine(std::string line) { - std::istringstream lineStream; SuppressionList::Suppression suppression; // Strip any end of line comments @@ -218,13 +217,18 @@ SuppressionList::Suppression SuppressionList::parseLine(const std::string &line) while (endpos > 0 && std::isspace(line[endpos-1])) { endpos--; } - lineStream.str(line.substr(0, endpos)); - } else { - lineStream.str(line); + line = line.substr(0, endpos); } - if (std::getline(lineStream, suppression.errorId, ':')) { - if (std::getline(lineStream, suppression.fileName)) { + const auto parts = splitString(line, '\n'); + const std::string& suppr_l = parts[0]; + + const std::string::size_type first_sep = suppr_l.find(':'); + suppression.errorId = suppr_l.substr(0, first_sep); + if (first_sep != std::string::npos) { + suppression.fileName = suppr_l.substr(first_sep+1); + if (!suppression.fileName.empty()) { + // TODO: this only works with files which have an extension // 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.. @@ -234,39 +238,48 @@ SuppressionList::Suppression SuppressionList::parseLine(const std::string &line) // if a colon is found and there is no dot after it.. if (pos != std::string::npos && - suppression.fileName.find('.', pos) == std::string::npos) { - // Try to parse out the line number - try { - std::istringstream istr1(suppression.fileName.substr(pos+1)); - istr1 >> suppression.lineNumber; - } catch (...) { - suppression.lineNumber = SuppressionList::Suppression::NO_LINE; - } + suppression.fileName.find('.', pos) == std::string::npos) + { + // parse out the line number + const std::string line_s = suppression.fileName.substr(pos+1); + suppression.fileName.erase(pos); - if (suppression.lineNumber != SuppressionList::Suppression::NO_LINE) { - suppression.fileName.erase(pos); - } - } + if (suppression.fileName.empty()) + throw std::runtime_error("filename is missing"); - // when parsing string generated internally by toString() there can be newline - std::string extra; - while (std::getline(lineStream, extra)) { - if (startsWith(extra, "symbol=")) - suppression.symbolName = extra.substr(7); - else if (extra == "polyspace=1") - suppression.isPolyspace = true; + try { + suppression.lineNumber = strToInt(line_s); + } catch (const std::runtime_error& e) { + throw std::runtime_error(std::string("invalid line number (") + e.what() + ")"); + } } + suppression.fileName = Path::simplifyPath(suppression.fileName); + } else { + throw std::runtime_error("filename is missing"); } } - suppression.fileName = Path::simplifyPath(suppression.fileName); + // TODO: make this optional - do we even encounter this in production code? + // when parsing string generated internally by toString() there can be newline + for (std::size_t i = 1; i < parts.size(); ++i) { + if (startsWith(parts[i], "symbol=")) + suppression.symbolName = parts[i].substr(7); + else if (parts[i] == "polyspace=1") + suppression.isPolyspace = true; + else + throw std::runtime_error("unexpected extra '" + parts[i] + "'"); + } return suppression; } std::string SuppressionList::addSuppressionLine(const std::string &line) { - return addSuppression(parseLine(line)); + try { + return addSuppression(parseLine(line)); + } catch (const std::exception& e) { + return e.what(); + } } std::string SuppressionList::addSuppression(SuppressionList::Suppression suppression) diff --git a/lib/suppressions.h b/lib/suppressions.h index 7f5ffb87b09..95f94a31af0 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -196,8 +196,9 @@ class CPPCHECKLIB SuppressionList { * Create a Suppression object from a suppression line * @param line The line to parse. * @return a suppression object + * @throws std::runtime_error thrown if the given suppression is invalid */ - static Suppression parseLine(const std::string &line); + static Suppression parseLine(std::string line); /** * @brief Don't show the given error. diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index b1f919b4b2a..72f6877d532 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -58,6 +58,7 @@ class TestSuppressions : public TestFixture { void run() override { mNewTemplate = true; TEST_CASE(parseLine); + TEST_CASE(parseLineInvalid); TEST_CASE(suppressionsBadId1); TEST_CASE(suppressionsDosFormat); // Ticket #1836 TEST_CASE(suppressionsFileNameWithColon); // Ticket #1919 - filename includes colon @@ -171,11 +172,39 @@ class TestSuppressions : public TestFixture { ASSERT_EQUALS("bad:test.c:1\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\nsymbol=x\npolyspace=1").toString()); // polyspace + symbol - ASSERT_EQUALS("bad\npolyspace=1\nsymbol=x", SuppressionList::parseLine("bad\npolyspace=1\nsymbol=x").toString()); + ASSERT_EQUALS("bad\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad\npolyspace=1\nsymbol=x").toString()); ASSERT_EQUALS("bad:test.c\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c\npolyspace=1\nsymbol=x").toString()); ASSERT_EQUALS("bad:test.c:1\nsymbol=x\npolyspace=1", SuppressionList::parseLine("bad:test.c:1\npolyspace=1\nsymbol=x").toString()); } + void parseLineInvalid() const { + // missing filename + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:"), std::runtime_error, "filename is missing"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:\n"), std::runtime_error, "filename is missing"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:\n1.c"), std::runtime_error, "filename is missing"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:#1.c"), std::runtime_error, "filename is missing"); // TODO: looks like a valid filename + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id://1.c"), std::runtime_error, "filename is missing"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id::"), std::runtime_error, "filename is missing"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id::1"), std::runtime_error, "filename is missing"); + + // missing/invalid line + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:\n"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:\n1"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:#1"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)"); // TODO: looks like a valid filename + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c://1"), std::runtime_error, "invalid line number (converting '' to integer failed - not an integer)"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:zero"), std::runtime_error, "invalid line number (converting 'zero' to integer failed - not an integer)"); + + // invalid extras + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\n"), std::runtime_error, "unexpected extra ''"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\nsym=x"), std::runtime_error, "unexpected extra 'sym=x'"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\nsymbol:x"), std::runtime_error, "unexpected extra 'symbol:x'"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\npolyspace=0"), std::runtime_error, "unexpected extra 'polyspace=0'"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c:1\npolyspace:1"), std::runtime_error, "unexpected extra 'polyspace:1'"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id\n:"), std::runtime_error, "unexpected extra ':'"); + ASSERT_THROW_EQUALS(SuppressionList::parseLine("id:1.c\n:"), std::runtime_error, "unexpected extra ':'"); + } + void suppressionsBadId1() const { SuppressionList suppressions; std::istringstream s1("123"); From 2f76affb2f08c137839ec3a67d6d6bd9f010950f Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Apr 2026 14:11:27 +0200 Subject: [PATCH 3/3] suppressions.cpp: fixed `uselessCallsSubstr` selfcheck warning --- lib/suppressions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index 577af9a9616..d14bcabc898 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -217,7 +217,7 @@ SuppressionList::Suppression SuppressionList::parseLine(std::string line) while (endpos > 0 && std::isspace(line[endpos-1])) { endpos--; } - line = line.substr(0, endpos); + line.resize(endpos); } const auto parts = splitString(line, '\n');