Skip to content

fixed platforms lookup / set proper platform type for unix32-unsigned and unix64-unsigned / copy platforms in CMake #4464

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
danmar merged 4 commits into from
Jan 26, 2023
Merged

fixed platforms lookup / set proper platform type for unix32-unsigned and unix64-unsigned / copy platforms in CMake #4464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dbe782b
set proper platform type for `unix32-unsigned` and `unix64-unsigned`
firewave Sep 15, 2022
1ddde10
added copying of `platforms` files to CMake
firewave Sep 17, 2022
53741bf
platforms.cpp: added missing paths in regular lookup
firewave Sep 17, 2022
b3dae01
added verbose logging to `Platform::loadPlatformFile()`
firewave Sep 17, 2022
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
5 changes: 5 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ add_custom_target(copy_addons ALL
"${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/addons"
COMMENT "Copying addons files to ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}")

add_custom_target(copy_platforms ALL
${CMAKE_COMMAND} -E copy_directory "${PROJECT_SOURCE_DIR}/platforms"
"${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}/platforms"
COMMENT "Copying platforms files to ${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/${CMAKE_CFG_INTDIR}")

if(USE_BUNDLED_TINYXML2)
message(STATUS "Using bundled version of tinyxml2")
add_subdirectory(externals/tinyxml2)
Expand Down
1 change: 1 addition & 0 deletions cli/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ target_link_libraries(cppcheck ${CMAKE_THREAD_LIBS_INIT})

add_dependencies(cppcheck copy_cfg)
add_dependencies(cppcheck copy_addons)
add_dependencies(cppcheck copy_platforms)
add_dependencies(cppcheck run-dmake)

install(TARGETS cppcheck
Expand Down
12 changes: 10 additions & 2 deletions cli/cmdlineparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,13 +584,21 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
mSettings->platform(Settings::Native);
else if (platform == "unspecified")
mSettings->platform(Settings::Unspecified);
else if (!mSettings->loadPlatformFile(argv[0], platform)) {
else if (!mSettings->loadPlatformFile(argv[0], platform, mSettings->verbose)) {
std::string message("unrecognized platform: \"");
message += platform;
message += "\".";
printError(message);
return false;
}

// TODO: remove
// these are loaded via external files and thus have Settings::PlatformFile set instead.
// override the type so they behave like the regular platforms.
if (platform == "unix32-unsigned")
mSettings->platformType = Settings::Unix32;
else if (platform == "unix64-unsigned")
mSettings->platformType = Settings::Unix64;
}

// Write results in results.plist
Expand Down Expand Up @@ -648,7 +656,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
mSettings->platform(Settings::Native);
else if (platform == "unspecified" || platform == "Unspecified" || platform.empty())
;
else if (!mSettings->loadPlatformFile(projectFile.c_str(), platform) && !mSettings->loadPlatformFile(argv[0], platform)) {
else if (!mSettings->loadPlatformFile(projectFile.c_str(), platform, mSettings->verbose) && !mSettings->loadPlatformFile(argv[0], platform, mSettings->verbose)) {
std::string message("unrecognized platform: \"");
message += platform;
message += "\".";
Expand Down
60 changes: 35 additions & 25 deletions lib/platform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "path.h"

#include <cstring>
#include <iostream>
#include <limits>
#include <vector>

Expand Down Expand Up @@ -156,36 +157,45 @@ bool cppcheck::Platform::platform(cppcheck::Platform::PlatformType type)
return false;
}

bool cppcheck::Platform::loadPlatformFile(const char exename[], const std::string &filename)
bool cppcheck::Platform::loadPlatformFile(const char exename[], const std::string &filename, bool verbose)
{
// open file..
tinyxml2::XMLDocument doc;
if (doc.LoadFile(filename.c_str()) != tinyxml2::XML_SUCCESS) {
std::vector<std::string> filenames;
filenames.push_back(filename + ".xml");
if (exename && (std::string::npos != Path::fromNativeSeparators(exename).find('/'))) {
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + filename);
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + filename);
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename);
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename + ".xml");
}
// TODO: only append .xml if missing
// TODO: use native separators
std::vector<std::string> filenames;
filenames.push_back(filename);
filenames.push_back(filename + ".xml");
filenames.push_back("platforms/" + filename);
filenames.push_back("platforms/" + filename + ".xml");
if (exename && (std::string::npos != Path::fromNativeSeparators(exename).find('/'))) {
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + filename);
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename);
filenames.push_back(Path::getPathFromFilename(Path::fromNativeSeparators(exename)) + "platforms/" + filename + ".xml");
}
#ifdef FILESDIR
std::string filesdir = FILESDIR;
if (!filesdir.empty() && filesdir[filesdir.size()-1] != '/')
filesdir += '/';
filenames.push_back(filesdir + ("platforms/" + filename));
filenames.push_back(filesdir + ("platforms/" + filename + ".xml"));
std::string filesdir = FILESDIR;
if (!filesdir.empty() && filesdir[filesdir.size()-1] != '/')
filesdir += '/';
filenames.push_back(filesdir + ("platforms/" + filename));
filenames.push_back(filesdir + ("platforms/" + filename + ".xml"));
#endif
bool success = false;
for (const std::string & f : filenames) {
if (doc.LoadFile(f.c_str()) == tinyxml2::XML_SUCCESS) {
success = true;
break;
}

// open file..
tinyxml2::XMLDocument doc;
bool success = false;
for (const std::string & f : filenames) {
if (verbose)
std::cout << "try to load platform file '" << f << "' ... ";
Copy link
Owner

@danmar danmar Oct 9, 2022

Choose a reason for hiding this comment

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

We should not output any important information with std::cout directly. Is it just for debugging? It will not be shown properly in the GUI.

Copy link
Collaborator Author

@firewave firewave Oct 9, 2022

Choose a reason for hiding this comment

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

It's verbose information. I just checked other code and that uses the ErrorLogger::reportOut(). Will adjust.

Copy link
Collaborator Author

@firewave firewave Nov 20, 2022

Choose a reason for hiding this comment

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

Turns out that is not the function to use since it is not available at that point.

Also the rest of the code is not doing this consistently either. So I propose to apply these changes as is. With a later PR I will refactor out the explicit std::cout and std::cerr usage as a whole.

if (doc.LoadFile(f.c_str()) == tinyxml2::XML_SUCCESS) {
if (verbose)
std::cout << "Success" << std::endl;
success = true;
break;
}
if (!success)
return false;
if (verbose)
std::cout << doc.ErrorStr() << std::endl;
}
if (!success)
return false;

return loadFromXmlDocument(&doc);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,10 @@ namespace cppcheck {
* load platform file
* @param exename application path
* @param filename platform filename
* @param verbose log verbose information about the lookup
* @return returns true if file was loaded successfully
*/
bool loadPlatformFile(const char exename[], const std::string &filename);
bool loadPlatformFile(const char exename[], const std::string &filename, bool verbose = false);

/** load platform from xml document, primarily for testing */
bool loadFromXmlDocument(const tinyxml2::XMLDocument *doc);
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ if (BUILD_TESTS)

add_dependencies(testrunner copy_cfg)
add_dependencies(testrunner copy_addons)
add_dependencies(testrunner copy_platforms)
add_dependencies(testrunner run-dmake)

if (LIBXML2_XMLLINT_EXECUTABLE)
Expand Down
35 changes: 26 additions & 9 deletions test/testcmdlineparser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,12 @@ class TestCmdlineParser : public TestFixture {
TEST_CASE(platformWin32A);
TEST_CASE(platformWin32W);
TEST_CASE(platformUnix32);
TEST_CASE(platformUnix32Unsigned);
TEST_CASE(platformUnix64);
TEST_CASE(platformUnix64Unsigned);
TEST_CASE(platformNative);
TEST_CASE(platformUnspecified);
//TEST_CASE(platformPlatformFile);
TEST_CASE(platformPlatformFile);
TEST_CASE(platformUnknown);
TEST_CASE(plistEmpty);
TEST_CASE(plistDoesNotExist);
Expand Down Expand Up @@ -903,6 +905,15 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS("", GET_REDIRECT_OUTPUT);
}

void platformUnix32Unsigned() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--platform=unix32-unsigned", "file.cpp"};
ASSERT(settings.platform(Settings::Unspecified));
ASSERT(defParser.parseFromArgs(3, argv));
ASSERT(settings.platformType == Settings::Unix32);
ASSERT_EQUALS("", GET_REDIRECT_OUTPUT);
}

void platformUnix64() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--platform=unix64", "file.cpp"};
Expand All @@ -912,6 +923,15 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS("", GET_REDIRECT_OUTPUT);
}

void platformUnix64Unsigned() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--platform=unix64-unsigned", "file.cpp"};
ASSERT(settings.platform(Settings::Unspecified));
ASSERT(defParser.parseFromArgs(3, argv));
ASSERT(settings.platformType == Settings::Unix64);
ASSERT_EQUALS("", GET_REDIRECT_OUTPUT);
}

void platformNative() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--platform=native", "file.cpp"};
Expand All @@ -930,17 +950,14 @@ class TestCmdlineParser : public TestFixture {
ASSERT_EQUALS("", GET_REDIRECT_OUTPUT);
}

/*
// TODO: the file is not found because of a bug in the lookup code
void platformPlatformFile() {
void platformPlatformFile() {
REDIRECT;
const char * const argv[] = {"cppcheck", "--platform=avr8", "file.cpp"};
ASSERT(settings.platform(Settings::Unspecified));
TODO_ASSERT_EQUALS(true, false, defParser.parseFromArgs(3, argv));
TODO_ASSERT_EQUALS(Settings::PlatformFile, Settings::Unspecified, settings.platformType);
TODO_ASSERT_EQUALS("cppcheck: error: unrecognized platform: \"avr8\".\n", "", GET_REDIRECT_OUTPUT);
}
*/
ASSERT_EQUALS(true, defParser.parseFromArgs(3, argv));
ASSERT_EQUALS(Settings::PlatformFile, settings.platformType);
ASSERT_EQUALS("", GET_REDIRECT_OUTPUT);
}

void platformUnknown() {
REDIRECT;
Expand Down