Skip to content

Commit

Permalink
generate AddonInfo only once (#4958)
Browse files Browse the repository at this point in the history
Currently the `AddonInfo` is generated and discarded on each addon
invocation. This leads to an unnecessary process invocation for each
addon on each file.

Also if an addon is completely broken we will still perform the whole
analysis only for it to be failed at the end so we should bail out early
if we know it doesn't work at all.
  • Loading branch information
firewave committed Oct 8, 2023
1 parent cc44966 commit 0f28f3e
Show file tree
Hide file tree
Showing 13 changed files with 365 additions and 247 deletions.
242 changes: 123 additions & 119 deletions Makefile

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions cli/cppcheckexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
if (!loadLibraries(settings))
return false;

if (!loadAddons(settings))
return false;

// Check that all include paths exist
{
for (std::list<std::string>::iterator iter = settings.includePaths.begin();
Expand Down Expand Up @@ -406,6 +409,22 @@ bool CppCheckExecutor::loadLibraries(Settings& settings)
return result;
}

bool CppCheckExecutor::loadAddons(Settings& settings)
{
bool result = true;
for (const std::string &addon: settings.addons) {
AddonInfo addonInfo;
const std::string failedToGetAddonInfo = addonInfo.getAddonInfo(addon, settings.exename);
if (!failedToGetAddonInfo.empty()) {
std::cout << failedToGetAddonInfo << std::endl;
result = false;
continue;
}
settings.addonInfos.emplace_back(std::move(addonInfo));
}
return result;
}

#ifdef _WIN32
// fix trac ticket #439 'Cppcheck reports wrong filename for filenames containing 8-bit ASCII'
static inline std::string ansiToOEM(const std::string &msg, bool doConvert)
Expand Down
7 changes: 7 additions & 0 deletions cli/cppcheckexecutor.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ class CppCheckExecutor : public ErrorLogger {
*/
bool loadLibraries(Settings& settings);

/**
* @brief Load addons
* @param settings Settings
* @return Returns true if successful
*/
static bool loadAddons(Settings& settings);

/**
* @brief Write the checkers report
*/
Expand Down
4 changes: 4 additions & 0 deletions gui/mainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,7 @@ Settings MainWindow::getCppcheckSettings()

addonFilePath.replace(QChar('\\'), QChar('/'));

// TODO: use picojson to generate the JSON
QString json;
json += "{ \"script\":\"" + addonFilePath + "\"";
if (!pythonCmd.isEmpty())
Expand All @@ -1014,6 +1015,9 @@ Settings MainWindow::getCppcheckSettings()
}
json += " }";
result.addons.emplace(json.toStdString());
AddonInfo addonInfo;
addonInfo.getAddonInfo(json.toStdString(), result.exename);
result.addonInfos.emplace_back(std::move(addonInfo));
}

if (isCppcheckPremium()) {
Expand Down
132 changes: 132 additions & 0 deletions lib/addoninfo.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2023 Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "addoninfo.h"

#include "path.h"
#include "utils.h"

#include <fstream>
#include <sstream>
#include <string>

#include "json.h"

static std::string getFullPath(const std::string &fileName, const std::string &exename) {
if (Path::isFile(fileName))
return fileName;

const std::string exepath = Path::getPathFromFilename(exename);
if (Path::isFile(exepath + fileName))
return exepath + fileName;
if (Path::isFile(exepath + "addons/" + fileName))
return exepath + "addons/" + fileName;

#ifdef FILESDIR
if (Path::isFile(FILESDIR + ("/" + fileName)))
return FILESDIR + ("/" + fileName);
if (Path::isFile(FILESDIR + ("/addons/" + fileName)))
return FILESDIR + ("/addons/" + fileName);
#endif
return "";
}

static std::string parseAddonInfo(AddonInfo& addoninfo, const picojson::value &json, const std::string &fileName, const std::string &exename) {
const std::string& json_error = picojson::get_last_error();
if (!json_error.empty()) {
return "Loading " + fileName + " failed. " + json_error;
}
if (!json.is<picojson::object>())
return "Loading " + fileName + " failed. Bad json.";
picojson::object obj = json.get<picojson::object>();
if (obj.count("args")) {
if (!obj["args"].is<picojson::array>())
return "Loading " + fileName + " failed. args must be array.";
for (const picojson::value &v : obj["args"].get<picojson::array>())
addoninfo.args += " " + v.get<std::string>();
}

if (obj.count("ctu")) {
// ctu is specified in the config file
if (!obj["ctu"].is<bool>())
return "Loading " + fileName + " failed. ctu must be boolean.";
addoninfo.ctu = obj["ctu"].get<bool>();
} else {
addoninfo.ctu = false;
}

if (obj.count("python")) {
// Python was defined in the config file
if (obj["python"].is<picojson::array>()) {
return "Loading " + fileName +" failed. python must not be an array.";
}
addoninfo.python = obj["python"].get<std::string>();
} else {
addoninfo.python = "";
}

if (obj.count("executable")) {
if (!obj["executable"].is<std::string>())
return "Loading " + fileName + " failed. executable must be a string.";
addoninfo.executable = getFullPath(obj["executable"].get<std::string>(), fileName);
return "";
}

return addoninfo.getAddonInfo(obj["script"].get<std::string>(), exename);
}

std::string AddonInfo::getAddonInfo(const std::string &fileName, const std::string &exename) {
if (fileName[0] == '{') {
picojson::value json;
const std::string err = picojson::parse(json, fileName);
(void)err; // TODO: report
return parseAddonInfo(*this, json, fileName, exename);
}
if (fileName.find('.') == std::string::npos)
return getAddonInfo(fileName + ".py", exename);

if (endsWith(fileName, ".py")) {
scriptFile = Path::fromNativeSeparators(getFullPath(fileName, exename));
if (scriptFile.empty())
return "Did not find addon " + fileName;

std::string::size_type pos1 = scriptFile.rfind('/');
if (pos1 == std::string::npos)
pos1 = 0;
else
pos1++;
std::string::size_type pos2 = scriptFile.rfind('.');
if (pos2 < pos1)
pos2 = std::string::npos;
name = scriptFile.substr(pos1, pos2 - pos1);

runScript = getFullPath("runaddon.py", exename);

return "";
}

if (!endsWith(fileName, ".json"))
return "Failed to open addon " + fileName;

std::ifstream fin(fileName);
if (!fin.is_open())
return "Failed to open " + fileName;
picojson::value json;
fin >> json;
return parseAddonInfo(*this, json, fileName, exename);
}
38 changes: 38 additions & 0 deletions lib/addoninfo.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2023 Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#ifndef addonInfoH
#define addonInfoH

#include "config.h"

#include <string>

struct CPPCHECKLIB AddonInfo {
std::string name;
std::string scriptFile; // addon script
std::string executable; // addon executable
std::string args; // special extra arguments
std::string python; // script interpreter
bool ctu = false;
std::string runScript;

std::string getAddonInfo(const std::string &fileName, const std::string &exename);
};

#endif // addonInfoH
130 changes: 6 additions & 124 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "cppcheck.h"

#include "addoninfo.h"
#include "check.h"
#include "checkunusedfunctions.h"
#include "clangimport.h"
Expand Down Expand Up @@ -105,122 +107,6 @@ namespace {
};
}

namespace {
struct AddonInfo {
std::string name;
std::string scriptFile; // addon script
std::string executable; // addon executable
std::string args; // special extra arguments
std::string python; // script interpreter
bool ctu = false;
std::string runScript;

static std::string getFullPath(const std::string &fileName, const std::string &exename) {
if (Path::isFile(fileName))
return fileName;

const std::string exepath = Path::getPathFromFilename(exename);
if (Path::isFile(exepath + fileName))
return exepath + fileName;
if (Path::isFile(exepath + "addons/" + fileName))
return exepath + "addons/" + fileName;

#ifdef FILESDIR
if (Path::isFile(FILESDIR + ("/" + fileName)))
return FILESDIR + ("/" + fileName);
if (Path::isFile(FILESDIR + ("/addons/" + fileName)))
return FILESDIR + ("/addons/" + fileName);
#endif
return "";
}

std::string parseAddonInfo(const picojson::value &json, const std::string &fileName, const std::string &exename) {
const std::string& json_error = picojson::get_last_error();
if (!json_error.empty()) {
return "Loading " + fileName + " failed. " + json_error;
}
if (!json.is<picojson::object>())
return "Loading " + fileName + " failed. Bad json.";
picojson::object obj = json.get<picojson::object>();
if (obj.count("args")) {
if (!obj["args"].is<picojson::array>())
return "Loading " + fileName + " failed. args must be array.";
for (const picojson::value &v : obj["args"].get<picojson::array>())
args += " " + v.get<std::string>();
}

if (obj.count("ctu")) {
// ctu is specified in the config file
if (!obj["ctu"].is<bool>())
return "Loading " + fileName + " failed. ctu must be boolean.";
ctu = obj["ctu"].get<bool>();
} else {
ctu = false;
}

if (obj.count("python")) {
// Python was defined in the config file
if (obj["python"].is<picojson::array>()) {
return "Loading " + fileName +" failed. python must not be an array.";
}
python = obj["python"].get<std::string>();
} else {
python = "";
}

if (obj.count("executable")) {
if (!obj["executable"].is<std::string>())
return "Loading " + fileName + " failed. executable must be a string.";
executable = getFullPath(obj["executable"].get<std::string>(), fileName);
return "";
}

return getAddonInfo(obj["script"].get<std::string>(), exename);
}

std::string getAddonInfo(const std::string &fileName, const std::string &exename) {
if (fileName[0] == '{') {
picojson::value json;
const std::string err = picojson::parse(json, fileName);
(void)err; // TODO: report
return parseAddonInfo(json, fileName, exename);
}
if (fileName.find('.') == std::string::npos)
return getAddonInfo(fileName + ".py", exename);

if (endsWith(fileName, ".py")) {
scriptFile = Path::fromNativeSeparators(getFullPath(fileName, exename));
if (scriptFile.empty())
return "Did not find addon " + fileName;

std::string::size_type pos1 = scriptFile.rfind('/');
if (pos1 == std::string::npos)
pos1 = 0;
else
pos1++;
std::string::size_type pos2 = scriptFile.rfind('.');
if (pos2 < pos1)
pos2 = std::string::npos;
name = scriptFile.substr(pos1, pos2 - pos1);

runScript = getFullPath("runaddon.py", exename);

return "";
}

if (!endsWith(fileName, ".json"))
return "Failed to open addon " + fileName;

std::ifstream fin(fileName);
if (!fin.is_open())
return "Failed to open " + fileName;
picojson::value json;
fin >> json;
return parseAddonInfo(json, fileName, exename);
}
};
}

static std::string cmdFileName(std::string f)
{
f = Path::toNativeSeparators(f);
Expand Down Expand Up @@ -1502,14 +1388,10 @@ void CppCheck::executeAddons(const std::vector<std::string>& files)
fout << f << std::endl;
}

for (const std::string &addon : mSettings.addons) {
AddonInfo addonInfo;
const std::string &failedToGetAddonInfo = addonInfo.getAddonInfo(addon, mSettings.exename);
if (!failedToGetAddonInfo.empty()) {
reportOut(failedToGetAddonInfo, Color::FgRed);
mExitCode = 1;
continue;
}
// ensure all addons have already been resolved - TODO: remove when settings are const after creation
assert(mSettings.addonInfos.size() == mSettings.addons.size());

for (const AddonInfo &addonInfo : mSettings.addonInfos) {
if (addonInfo.name != "misra" && !addonInfo.ctu && endsWith(files.back(), ".ctu-info"))
continue;

Expand Down
Loading

1 comment on commit 0f28f3e

@danmar
Copy link
Owner

@danmar danmar commented on 0f28f3e Nov 29, 2023

Choose a reason for hiding this comment

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

@firewave I believe this caused a regression when running misra addon in the GUI. See https://trac.cppcheck.net/ticket/12227

Please sign in to comment.