Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 41 additions & 28 deletions lib/suppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,8 @@ std::vector<SuppressionList::Suppression> 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
Expand All @@ -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.resize(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..
Expand All @@ -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;
}
Comment on lines -239 to -244
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try-catch did nothing because the stream was not set to throw exceptions.

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<int>(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?
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines with newlines can/are not being supported via suppression files and the --suppress option so we should disable support in those cases. After that there is not much left which parses lines.

I will look further into this in a follow-up.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are used in the IPC data and could be used in cppcheck.cfg. In all other cases we should be able to disable this.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newlines were only recently introduced in #8169 which is a very unfortunate change. It would make more sense to preserve the previous : separator and make the additional fields separated by something like , or ;.

// 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)
Expand Down
3 changes: 2 additions & 1 deletion lib/suppressions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
50 changes: 50 additions & 0 deletions test/testsuppressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -146,13 +147,62 @@ 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\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 {
Expand Down
Loading