Skip to content

Commit

Permalink
made Library::load(const tinyxml2::XMLDocument &) private / related…
Browse files Browse the repository at this point in the history 
… refactorings
  • Loading branch information
@firewave
firewave committed May 23, 2024
1 parent ab69847 commit 06eba23
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 60 deletions.
4 changes: 2 additions & 2 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,10 +682,10 @@ cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h
cli/threadexecutor.o: cli/threadexecutor.cpp cli/executor.h cli/threadexecutor.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/threadexecutor.cpp

test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h lib/xml.h test/fixture.h test/options.h test/redirect.h
test/fixture.o: test/fixture.cpp externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h test/options.h test/redirect.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/fixture.cpp

test/helpers.o: test/helpers.cpp cli/filelister.h externals/simplecpp/simplecpp.h lib/addoninfo.h lib/config.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/helpers.h
test/helpers.o: test/helpers.cpp cli/filelister.h externals/simplecpp/simplecpp.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/config.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/helpers.h
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/helpers.cpp

test/main.o: test/main.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h test/options.h
Expand Down
9 changes: 3 additions & 6 deletions lib/library.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ namespace tinyxml2 {
* @brief Library definitions handling
*/
class CPPCHECKLIB Library {
// TODO: get rid of this
friend class TestSymbolDatabase; // For testing only
friend class TestSingleExecutorBase; // For testing only
friend class TestThreadExecutorBase; // For testing only
friend class TestProcessExecutorBase; // For testing only
friend struct LibraryHelper; // for testing

public:
Library() = default;
Expand All @@ -76,7 +72,6 @@ class CPPCHECKLIB Library {
};

Error load(const char exename[], const char path[]);
Error load(const tinyxml2::XMLDocument &doc);

struct AllocFunc {
int groupId;
Expand Down Expand Up @@ -466,6 +461,8 @@ class CPPCHECKLIB Library {
bool hasAnyTypeCheck(const std::string& typeName) const;

private:
Error load(const tinyxml2::XMLDocument &doc);

// load a <function> xml node
Error loadFunction(const tinyxml2::XMLElement * const node, const std::string &name, std::set<std::string> &unknown_elements);

Expand Down
5 changes: 3 additions & 2 deletions test/fixture.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "cppcheck.h"
#include "errortypes.h"
#include "helpers.h"
#include "options.h"
#include "redirect.h"

Expand Down Expand Up @@ -484,8 +485,8 @@ TestFixture::SettingsBuilder& TestFixture::SettingsBuilder::libraryxml(const cha
tinyxml2::XMLDocument doc;
const tinyxml2::XMLError xml_error = doc.Parse(xmldata, len);
if (tinyxml2::XML_SUCCESS != xml_error)
throw std::runtime_error(std::string("loading XML data failed - ") + tinyxml2::XMLDocument::ErrorIDToName(xml_error));
const Library::ErrorCode lib_error = settings.library.load(doc).errorcode;
throw std::runtime_error(std::string("loading library XML data failed - ") + tinyxml2::XMLDocument::ErrorIDToName(xml_error));
const Library::ErrorCode lib_error = LibraryHelper::loadxmldoc(settings.library, doc).errorcode;
if (lib_error != Library::ErrorCode::OK)
throw std::runtime_error("loading library XML failed - " + std::to_string(static_cast<int>(lib_error)));
return *this;
Expand Down
23 changes: 23 additions & 0 deletions test/helpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "filelister.h"
#include "filesettings.h"
#include "library.h"
#include "path.h"
#include "pathmatch.h"
#include "preprocessor.h"
Expand All @@ -43,6 +44,8 @@

#include <simplecpp.h>

#include "xml.h"

class SuppressionList;

const Settings SimpleTokenizer::s_settings;
Expand Down Expand Up @@ -182,3 +185,23 @@ void PreprocessorHelper::preprocess(const char code[], std::vector<std::string>
std::list<Directive> directives = preprocessor.createDirectives(tokens1);
tokenizer.setDirectives(std::move(directives));
}

bool LibraryHelper::loadxmldata(Library &lib, const char xmldata[], std::size_t len)
{
tinyxml2::XMLDocument doc;
return (tinyxml2::XML_SUCCESS == doc.Parse(xmldata, len)) && (lib.load(doc).errorcode == Library::ErrorCode::OK);
}

bool LibraryHelper::loadxmldata(Library &lib, Library::Error& liberr, const char xmldata[], std::size_t len)
{
tinyxml2::XMLDocument doc;
if (tinyxml2::XML_SUCCESS != doc.Parse(xmldata, len))
return false;
liberr = lib.load(doc);
return true;
}

Library::Error LibraryHelper::loadxmldoc(Library &lib, const tinyxml2::XMLDocument& doc)
{
return lib.load(doc);
}
7 changes: 7 additions & 0 deletions test/helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,11 @@ inline std::string filter_valueflow(const std::string& s) {
return ostr;
}

struct LibraryHelper
{
static bool loadxmldata(Library &lib, const char xmldata[], std::size_t len);
static bool loadxmldata(Library &lib, Library::Error& liberr, const char xmldata[], std::size_t len);
static Library::Error loadxmldoc(Library &lib, const tinyxml2::XMLDocument& doc);
};

#endif // helpersH
75 changes: 33 additions & 42 deletions test/testlibrary.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ class TestLibrary : public TestFixture {
TEST_CASE(loadLibCombinations);
}

static bool loadxmldata(Library &lib, const char xmldata[], std::size_t len)
{
tinyxml2::XMLDocument doc;
return (tinyxml2::XML_SUCCESS == doc.Parse(xmldata, len)) && (lib.load(doc).errorcode == Library::ErrorCode::OK);
}

void isCompliantValidationExpression() const {
ASSERT_EQUALS(true, Library::isCompliantValidationExpression("-1"));
ASSERT_EQUALS(true, Library::isCompliantValidationExpression("1"));
Expand All @@ -104,7 +98,7 @@ class TestLibrary : public TestFixture {
// Reading an empty library file is considered to be OK
constexpr char xmldata[] = "<?xml version=\"1.0\"?>\n<def/>";
Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(library.functions().empty());
}

Expand All @@ -121,7 +115,7 @@ class TestLibrary : public TestFixture {
tokenList.front()->next()->astOperand1(tokenList.front());

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT_EQUALS(library.functions().size(), 1U);
ASSERT(library.functions().at("foo").argumentChecks.empty());
ASSERT(library.isnotnoreturn(tokenList.front()));
Expand All @@ -136,7 +130,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
{
const char code[] = "fred.foo(123);"; // <- wrong scope, not library function
const SimpleTokenList tokenList(code);
Expand Down Expand Up @@ -166,7 +160,7 @@ class TestLibrary : public TestFixture {
tokenList.createAst();

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(library.isNotLibraryFunction(tokenList.front()));
}

Expand All @@ -180,7 +174,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

{
TokenList tokenList(&settings);
Expand Down Expand Up @@ -234,7 +228,7 @@ class TestLibrary : public TestFixture {
tokenList.front()->next()->varId(1);

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(library.isNotLibraryFunction(tokenList.front()->next()));
}

Expand All @@ -251,7 +245,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT_EQUALS(0, library.functions().at("foo").argumentChecks.at(1).notuninit);
ASSERT_EQUALS(true, library.functions().at("foo").argumentChecks.at(2).notnull);
ASSERT_EQUALS(true, library.functions().at("foo").argumentChecks.at(3).formatstr);
Expand All @@ -270,7 +264,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT_EQUALS(0, library.functions().at("foo").argumentChecks.at(-1).notuninit);
}

Expand All @@ -284,7 +278,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT_EQUALS(0, library.functions().at("foo").argumentChecks.at(-1).notuninit);

const char code[] = "foo(a,b,c,d,e);";
Expand All @@ -309,7 +303,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

const char code[] = "foo(a,b,c,d);";
SimpleTokenList tokenList(code);
Expand Down Expand Up @@ -340,7 +334,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

const char code[] = "foo(a,b,c,d,e,f,g,h,i,j,k);";
SimpleTokenList tokenList(code);
Expand Down Expand Up @@ -481,7 +475,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

const char code[] = "foo(a,b,c,d,e);";
SimpleTokenList tokenList(code);
Expand Down Expand Up @@ -539,7 +533,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT_EQUALS(library.functions().size(), 2U);
ASSERT(library.functions().at("Foo::foo").argumentChecks.empty());
ASSERT(library.functions().at("bar").argumentChecks.empty());
Expand All @@ -566,7 +560,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT_EQUALS(library.functions().size(), 1U);

{
Expand All @@ -593,7 +587,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

{
SimpleTokenizer tokenizer(settings, *this);
Expand Down Expand Up @@ -622,7 +616,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

const char code[] = "a(); b();";
const SimpleTokenList tokenList(code);
Expand Down Expand Up @@ -655,7 +649,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(library.functions().empty());

const Library::AllocFunc* af = library.getAllocFuncInfo("CreateX");
Expand All @@ -682,8 +676,8 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT_EQUALS(true, loadxmldata(library, xmldata1, sizeof(xmldata1)));
ASSERT_EQUALS(true, loadxmldata(library, xmldata2, sizeof(xmldata2)));
ASSERT_EQUALS(true, LibraryHelper::loadxmldata(library, xmldata1, sizeof(xmldata1)));
ASSERT_EQUALS(true, LibraryHelper::loadxmldata(library, xmldata2, sizeof(xmldata2)));

ASSERT_EQUALS(library.deallocId("free"), library.allocId("malloc"));
ASSERT_EQUALS(library.deallocId("free"), library.allocId("foo"));
Expand All @@ -698,7 +692,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(library.functions().empty());

const Library::AllocFunc* af = library.getAllocFuncInfo("CreateX");
Expand All @@ -717,7 +711,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(library.functions().empty());

ASSERT(Library::isresource(library.allocId("CreateX")));
Expand All @@ -734,7 +728,7 @@ class TestLibrary : public TestFixture {
" <podtype name=\"s16\" sign=\"s\" size=\"2\"/>\n"
"</def>";
Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
// s8
{
const Library::PodType * const type = library.podtype("s8");
Expand Down Expand Up @@ -813,7 +807,7 @@ class TestLibrary : public TestFixture {
"</def>";

Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));

const Library::Container& A = library.containers().at("A");
const Library::Container& B = library.containers().at("B");
Expand Down Expand Up @@ -923,43 +917,40 @@ class TestLibrary : public TestFixture {
}
}

#define LOADLIBERROR(xmldata, errorcode) loadLibError(xmldata, errorcode, __FILE__, __LINE__)

void version() const {
{
constexpr char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def>\n"
"</def>";
Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
}
{
constexpr char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def format=\"1\">\n"
"</def>";
Library library;
ASSERT(loadxmldata(library, xmldata, sizeof(xmldata)));
ASSERT(LibraryHelper::loadxmldata(library, xmldata, sizeof(xmldata)));
}
{
constexpr char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def format=\"42\">\n"
"</def>";
Library library;
const Library::Error err = readLibrary(library, xmldata);
ASSERT_EQUALS(true, err.errorcode == Library::ErrorCode::UNSUPPORTED_FORMAT);
LOADLIBERROR(xmldata, Library::ErrorCode::UNSUPPORTED_FORMAT);
}
}

static Library::Error readLibrary(Library& library, const char* xmldata) {
tinyxml2::XMLDocument doc;
doc.Parse(xmldata); // TODO: check result
return library.load(doc);
}

void loadLibError(const char xmldata[], Library::ErrorCode errorcode, const char* file, unsigned line) const {
template<std::size_t size>
void loadLibError(const char (&xmldata)[size], Library::ErrorCode errorcode, const char* file, unsigned line) const {
Library library;
assertEquals(file, line, true, errorcode == readLibrary(library, xmldata).errorcode);
Library::Error liberr;
assertEquals(file, line, true, LibraryHelper::loadxmldata(library, liberr, xmldata, size-1));
assertEquals(file, line, true, errorcode == liberr.errorcode);
}

#define LOADLIBERROR(xmldata, errorcode) loadLibError(xmldata, errorcode, __FILE__, __LINE__)
#define LOADLIB_ERROR_INVALID_RANGE(valid) LOADLIBERROR("<?xml version=\"1.0\"?>\n" \
"<def>\n" \
"<function name=\"f\">\n" \
Expand Down
Loading

0 comments on commit 06eba23

Please sign in to comment.