Skip to content

improved library loading in tests #4806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 2, 2023
Merged

improved library loading in tests #4806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6cf3981
added missing XML and library loading asserts in tests / cleanups
firewave Feb 16, 2023
b9b8e5b
Settings: added generic `hasLib()` and removed `posix()`
firewave Feb 17, 2023
ee62bfc
aligned `Settings::library` and `Settings::libraries` in tests / adde…
firewave Feb 17, 2023
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
9 changes: 1 addition & 8 deletions lib/checkclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,6 @@ static bool isVariableCopyNeeded(const Variable &var, Function::Type type)
(var.valueType() && var.valueType()->type >= ValueType::Type::CHAR));
}

static bool isVcl(const Settings *settings)
{
return std::any_of(settings->libraries.cbegin(), settings->libraries.cend(), [](const std::string& library) {
return library == "vcl";
});
}

static bool isVclTypeInit(const Type *type)
{
if (!type)
Expand Down Expand Up @@ -141,7 +134,7 @@ void CheckClass::constructors()

const bool printInconclusive = mSettings->certainty.isEnabled(Certainty::inconclusive);
for (const Scope * scope : mSymbolDatabase->classAndStructScopes) {
if (isVcl(mSettings) && isVclTypeInit(scope->definedType))
if (mSettings->hasLib("vcl") && isVclTypeInit(scope->definedType))
continue;

const bool unusedTemplate = Token::simpleMatch(scope->classDef->previous(), ">");
Expand Down
6 changes: 3 additions & 3 deletions lib/checkmemoryleak.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getAllocationType(const Token *tok2,
return New;
}

if (mSettings_->posix()) {
if (mSettings_->hasLib("posix")) {
if (Token::Match(tok2, "open|openat|creat|mkstemp|mkostemp|socket (")) {
// simple sanity check of function parameters..
// TODO: Make such check for all these functions
Expand Down Expand Up @@ -237,7 +237,7 @@ CheckMemoryLeak::AllocType CheckMemoryLeak::getDeallocationType(const Token *tok
if (tok->str() == "realloc" && Token::simpleMatch(vartok->next(), ", 0 )"))
return Malloc;

if (mSettings_->posix()) {
if (mSettings_->hasLib("posix")) {
if (tok->str() == "close")
return Fd;
if (tok->str() == "pclose")
Expand Down Expand Up @@ -276,7 +276,7 @@ bool CheckMemoryLeak::isReopenStandardStream(const Token *tok) const

bool CheckMemoryLeak::isOpenDevNull(const Token *tok) const
{
if (mSettings_->posix() && tok->str() == "open" && numberOfArguments(tok) == 2) {
if (mSettings_->hasLib("posix") && tok->str() == "open" && numberOfArguments(tok) == 2) {
const Token* arg = getArguments(tok).at(0);
if (Token::simpleMatch(arg, "\"/dev/null\""))
return true;
Expand Down
6 changes: 3 additions & 3 deletions lib/settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,9 +417,9 @@ class CPPCHECKLIB Settings : public cppcheck::Platform {
*/
bool isEnabled(const ValueFlow::Value *value, bool inconclusiveCheck=false) const;

/** Is posix library specified? */
bool posix() const {
return std::find(libraries.cbegin(), libraries.cend(), "posix") != libraries.cend();
/** Is library specified? */
bool hasLib(const std::string &lib) const {
return std::find(libraries.cbegin(), libraries.cend(), lib) != libraries.cend();
}

/** @brief Request termination of checking */
Expand Down
1 change: 1 addition & 0 deletions test/testautovariables.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class TestAutoVariables : public TestFixture {
settings.severity.enable(Severity::style);
LOAD_LIB_2(settings.library, "std.cfg");
LOAD_LIB_2(settings.library, "qt.cfg");
settings.libraries.emplace_back("qt");

TEST_CASE(testautovar1);
TEST_CASE(testautovar2);
Expand Down
21 changes: 6 additions & 15 deletions test/testbufferoverrun.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3528,9 +3528,7 @@ class TestBufferOverrun : public TestFixture {
" <arg nr=\"2\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));

// Attempt to get size from Cfg files, no false positives if size is not specified
check("void f() {\n"
Expand Down Expand Up @@ -4083,9 +4081,7 @@ class TestBufferOverrun : public TestFixture {
" <arg nr=\"3\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));
settings.severity.enable(Severity::warning);
settings.sizeof_wchar_t = 4;

Expand Down Expand Up @@ -4225,9 +4221,7 @@ class TestBufferOverrun : public TestFixture {
" <arg nr=\"3\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));

check("void f() {\n"
" char c[7];\n"
Expand Down Expand Up @@ -4289,9 +4283,7 @@ class TestBufferOverrun : public TestFixture {
" </arg>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));

// formatstr..
check("void f() {\n"
Expand Down Expand Up @@ -4403,9 +4395,7 @@ class TestBufferOverrun : public TestFixture {
" <arg nr=\"4\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));

check("void f() {\n"
" char c[5];\n"
Expand Down Expand Up @@ -5498,6 +5488,7 @@ class TestBufferOverrun : public TestFixture {

Settings settings;
LOAD_LIB_2(settings.library, "posix.cfg");
settings.libraries.emplace_back("posix");

check("void f(){\n"
"int pipefd[1];\n" // <-- array of two integers is needed
Expand Down
10 changes: 3 additions & 7 deletions test/testclass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,8 @@ class TestClass : public TestFixture {
" </access>\n"
" </container>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings0.library.load(doc);
settings1.library.load(doc);
ASSERT(settings0.library.loadxmldata(xmldata, sizeof(xmldata)));
ASSERT(settings1.library.loadxmldata(xmldata, sizeof(xmldata)));
}


Expand Down Expand Up @@ -3390,9 +3388,7 @@ class TestClass : public TestFixture {
" <podtype name=\"std::uint8_t\" sign=\"u\" size=\"1\"/>\n"
" <podtype name=\"std::atomic_bool\"/>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));

checkNoMemset("class A {\n"
" std::array<int, 10> ints;\n"
Expand Down
5 changes: 2 additions & 3 deletions test/testcondition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class TestCondition : public TestFixture {
PLATFORM(settings1, cppcheck::Platform::Native);

LOAD_LIB_2(settings0.library, "qt.cfg");
settings0.libraries.emplace_back("qt");
LOAD_LIB_2(settings0.library, "std.cfg");

settings0.severity.enable(Severity::style);
Expand All @@ -58,11 +59,9 @@ class TestCondition : public TestFixture {
"<def>\n"
" <function name=\"bar\"> <pure/> </function>\n"
"</def>";
tinyxml2::XMLDocument xmldoc;
xmldoc.Parse(cfg, sizeof(cfg));
ASSERT(settings1.library.loadxmldata(cfg, sizeof(cfg)));
settings1.severity.enable(Severity::style);
settings1.severity.enable(Severity::warning);
settings1.library.load(xmldoc);

TEST_CASE(assignAndCompare); // assignment and comparison don't match
TEST_CASE(mismatchingBitAnd); // overlapping bitmasks
Expand Down
7 changes: 5 additions & 2 deletions test/testconstructors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1952,7 +1952,8 @@ class TestConstructors : public TestFixture {

void initvar_smartptr() { // #10237
Settings s;
s.libraries.emplace_back("std");
// TODO: test shuld probably not pass without library
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chrchr-github Please have a look at this. It was added in 9d6e5c2.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That should probably be LOAD_LIB_2(s.library, "std.cfg"); Since the fix was for a FP, the test is ineffective, but still passes with or without the library

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in that commit check for smart pointers. But without the library it shouldn't know what is a smart pointer.

Let me guess - those are tracked hard-coded in the source instead of using the library.

//LOAD_LIB_2(s.library, "std.cfg");
check("struct S {\n"
" explicit S(const std::shared_ptr<S>& sp) {\n"
" set(*sp);\n"
Expand Down Expand Up @@ -3589,7 +3590,9 @@ class TestConstructors : public TestFixture {

void uninitVarInheritClassInit() {
Settings s;
s.libraries.emplace_back("vcl");
// TODO: test should probably not pass without library
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@danmar Please have a look at this. It was added in 03445c0.

Copy link
Owner

Choose a reason for hiding this comment

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

Loading a library with the name "vcl" triggers a bit of hard coded handling in cppcheck. That is unfortunate. But I think the code is intentional at least.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but the test never triggers that code since it does pass without the library being loaded.

Copy link
Owner

Choose a reason for hiding this comment

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

hmm I believe I wanted to test that there would be no FP. I am confused why you could remove the vcl library and still don't get false positives.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Please take a look. It doesn't have to be tackled within this PR though.

//LOAD_LIB_2(s.library, "vcl.cfg");
//s.libraries.emplace_back("vcl");

check("class Fred: public TObject\n"
"{\n"
Expand Down
2 changes: 1 addition & 1 deletion test/testerrorlogger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ class TestErrorLogger : public TestFixture {
" <location file=\"foo.cpp\" line=\"5\" column=\"2\"/>\n"
"</error>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
ASSERT(doc.Parse(xmldata, sizeof(xmldata)) == tinyxml2::XML_SUCCESS);
ErrorMessage msg(doc.FirstChildElement());
ASSERT_EQUALS("errorId", msg.id);
ASSERT_EQUALS(Severity::error, msg.severity);
Expand Down
10 changes: 3 additions & 7 deletions test/testfunctions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ class TestFunctions : public TestFixture {
settings.severity.enable(Severity::performance);
settings.severity.enable(Severity::portability);
settings.certainty.enable(Certainty::inconclusive);
settings.libraries.emplace_back("posix");
settings.standards.c = Standards::C11;
settings.standards.cpp = Standards::CPP11;
LOAD_LIB_2(settings.library, "std.cfg");
LOAD_LIB_2(settings.library, "posix.cfg");
settings.libraries.emplace_back("posix");

// Prohibited functions
TEST_CASE(prohibitedFunctions_posix);
Expand Down Expand Up @@ -1313,9 +1313,7 @@ class TestFunctions : public TestFixture {
" <arg nr=\"2\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings2.library.load(doc);
ASSERT(settings2.library.loadxmldata(xmldata, sizeof(xmldata)));

check("void foo() {\n"
" mystrcmp(a, b);\n"
Expand Down Expand Up @@ -1468,9 +1466,7 @@ class TestFunctions : public TestFixture {
" <arg nr=\"2\"/>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings2.library.load(doc);
ASSERT(settings2.library.loadxmldata(xmldata, sizeof(xmldata)));

check("void foo() {\n"
" mystrcmp(a, b);\n"
Expand Down
1 change: 1 addition & 0 deletions test/testio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class TestIO : public TestFixture {
LOAD_LIB_2(settings.library, "std.cfg");
LOAD_LIB_2(settings.library, "windows.cfg");
LOAD_LIB_2(settings.library, "qt.cfg");
settings.libraries.emplace_back("qt");

TEST_CASE(coutCerrMisusage);

Expand Down
14 changes: 5 additions & 9 deletions test/testleakautovar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ class TestLeakAutoVar : public TestFixture {
"<def>\n"
" <podtype name=\"uint8_t\" sign=\"u\" size=\"1\"/>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settings.library.load(doc);
ASSERT(settings.library.loadxmldata(xmldata, sizeof(xmldata)));

// Assign
TEST_CASE(assign1);
Expand Down Expand Up @@ -474,6 +472,7 @@ class TestLeakAutoVar : public TestFixture {
void assign23() {
Settings s = settings;
LOAD_LIB_2(settings.library, "posix.cfg");
settings.libraries.emplace_back("posix");
check("void f() {\n"
" int n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11, n12, n13, n14;\n"
" *&n1 = open(\"xx.log\", O_RDONLY);\n"
Expand Down Expand Up @@ -1854,6 +1853,7 @@ class TestLeakAutoVar : public TestFixture {
Settings s;
LOAD_LIB_2(s.library, "std.cfg");
LOAD_LIB_2(s.library, "posix.cfg");
s.libraries.emplace_back("posix");

check("void f() {\n"
" char* temp = strdup(\"temp.txt\");\n"
Expand Down Expand Up @@ -2594,9 +2594,7 @@ class TestLeakAutoVar : public TestFixture {
" </arg>\n"
" </function>\n"
"</def>";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settingsFunctionCall.library.load(doc);
ASSERT(settingsFunctionCall.library.loadxmldata(xmldata, sizeof(xmldata)));
check("void test_func()\n"
"{\n"
" char * buf = malloc(4);\n"
Expand All @@ -2616,9 +2614,7 @@ class TestLeakAutoVar : public TestFixture {
" <arg nr=\"1\" direction=\"in\"/>\n"
" </function>\n"
"</def>\n";
tinyxml2::XMLDocument doc;
doc.Parse(xmldata, sizeof(xmldata));
settingsLeakIgnore.library.load(doc);
ASSERT(settingsLeakIgnore.library.loadxmldata(xmldata, sizeof(xmldata)));
check("void f() {\n"
" double* a = new double[1024];\n"
" SomeClass::someMethod(a);\n"
Expand Down
Loading