From 32e569704bd12de6dc00d1886883dece9ac63288 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 19 May 2020 16:04:25 +0200 Subject: [PATCH] executeCommand in CppCheckExecutor or QCheckThread --- Makefile | 2 +- cli/cppcheckexecutor.cpp | 35 ++++++- cli/cppcheckexecutor.h | 5 + cli/threadexecutor.cpp | 2 +- gui/checkthread.cpp | 27 +++++- gui/mainwindow.cpp | 2 +- gui/newsuppressiondialog.cpp | 2 +- lib/cppcheck.cpp | 180 ++++++++++++++++------------------- lib/cppcheck.h | 7 +- test/testcppcheck.cpp | 2 +- test/testsuppressions.cpp | 6 +- 11 files changed, 160 insertions(+), 110 deletions(-) diff --git a/Makefile b/Makefile index b80adae2d..5996830b8 100644 --- a/Makefile +++ b/Makefile @@ -496,7 +496,7 @@ $(libcppdir)/ctu.o: lib/ctu.cpp externals/tinyxml/tinyxml2.h lib/astutils.h lib/ $(libcppdir)/errorlogger.o: lib/errorlogger.cpp externals/tinyxml/tinyxml2.h lib/analyzerinfo.h lib/check.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/errorlogger.o $(libcppdir)/errorlogger.cpp -$(libcppdir)/exprengine.o: lib/exprengine.cpp lib/astutils.h lib/config.h lib/errorlogger.h lib/exprengine.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h +$(libcppdir)/exprengine.o: lib/exprengine.cpp externals/z3_version.h lib/astutils.h lib/config.h lib/errorlogger.h lib/exprengine.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/exprengine.o $(libcppdir)/exprengine.cpp $(libcppdir)/forwardanalyzer.o: lib/forwardanalyzer.cpp lib/astutils.h lib/config.h lib/errorlogger.h lib/forwardanalyzer.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/utils.h lib/valueflow.h lib/valueptr.h diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 14b2074a3..ff7f01237 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -209,7 +209,7 @@ int CppCheckExecutor::check(int argc, const char* const argv[]) CheckUnusedFunctions::clear(); - CppCheck cppCheck(*this, true); + CppCheck cppCheck(*this, true, executeCommand); const Settings& settings = cppCheck.settings(); mSettings = &settings; @@ -1162,3 +1162,36 @@ bool CppCheckExecutor::tryLoadLibrary(Library& destination, const char* basepath } return true; } + +/** + * Execute a shell command and read the output from it. Returns true if command terminated successfully. + */ +bool CppCheckExecutor::executeCommand(std::string exe, std::vector args, std::string redirect, std::string *output) +{ + output->clear(); + + std::string joinedArgs; + for (const std::string arg: args) { + if (!joinedArgs.empty()) + joinedArgs += " "; + joinedArgs += arg; + } + +#ifdef _WIN32 + // Extra quoutes are needed in windows if filename has space + if (exe.find(" ") != std::string::npos) + exe = "\"" + exe + "\""; + const std::string cmd = exe + " " + joinedArgs + " " + redirect; + std::unique_ptr pipe(_popen(cmd.c_str(), "r"), _pclose); +#else + const std::string cmd = exe + " " + joinedArgs + " " + redirect; + std::unique_ptr pipe(popen(cmd.c_str(), "r"), pclose); +#endif + if (!pipe) + return false; + char buffer[1024]; + while (fgets(buffer, sizeof(buffer), pipe.get()) != nullptr) + *output += buffer; + return true; +} + diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index b9725ed84..050a35da6 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -108,6 +108,11 @@ public: */ static bool tryLoadLibrary(Library& destination, const char* basepath, const char* filename); + /** + * Execute a shell command and read the output from it. Returns true if command terminated successfully. + */ + static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string *output); + protected: /** diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index eb2306e1d..7469137fb 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -213,7 +213,7 @@ unsigned int ThreadExecutor::check() close(pipes[0]); mWpipe = pipes[1]; - CppCheck fileChecker(*this, false); + CppCheck fileChecker(*this, false, CppCheckExecutor::executeCommand); fileChecker.settings() = mSettings; unsigned int resultOfCheck = 0; diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index 37f606bd4..db271b5b3 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -32,10 +32,35 @@ #include "cppcheck.h" #include "common.h" +static bool executeCommand(std::string exe, std::vector args, std::string redirect, std::string *output) +{ + output->clear(); + + QStringList args2; + for (std::string arg: args) + args2 << QString::fromStdString(arg); + + QProcess process; + process.start(QString::fromStdString(exe), args2); + process.waitForFinished(); + + if (redirect == "2>&1") + *output = process.readAll().toStdString(); + else + *output = process.readAllStandardOutput().toStdString(); + + if (redirect.compare(0,3,"2> ") == 0) { + std::ofstream fout(redirect.substr(3)); + fout << process.readAllStandardError().toStdString(); + } + return true; +} + + CheckThread::CheckThread(ThreadResult &result) : mState(Ready), mResult(result), - mCppcheck(result, true), + mCppcheck(result, true, executeCommand), mAnalyseWholeProgram(false) { //ctor diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index 1e28cf3fe..1e5655112 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -536,7 +536,7 @@ void MainWindow::analyzeCode(const QString& code, const QString& filename) mUI.mResults, SLOT(debugError(const ErrorItem &))); // Create CppCheck instance - CppCheck cppcheck(result, true); + CppCheck cppcheck(result, true, nullptr); cppcheck.settings() = getCppcheckSettings(); // Check diff --git a/gui/newsuppressiondialog.cpp b/gui/newsuppressiondialog.cpp index fe38c8ed9..6f7039aa0 100644 --- a/gui/newsuppressiondialog.cpp +++ b/gui/newsuppressiondialog.cpp @@ -21,7 +21,7 @@ NewSuppressionDialog::NewSuppressionDialog(QWidget *parent) : }; QErrorLogger errorLogger; - CppCheck cppcheck(errorLogger,false); + CppCheck cppcheck(errorLogger, false, nullptr); cppcheck.getErrorMessages(); errorLogger.errorIds.sort(); diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 0bf804e02..f4784f319 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -164,73 +164,7 @@ static std::string cmdFileName(std::string f) return f; } -static bool executeShellCommand(std::string exe, const std::string &args, std::string *output) -{ - output->clear(); - -#ifdef _WIN32 - // Extra quoutes are needed in windows if filename has space - if (exe.find(" ") != std::string::npos) - exe = "\"" + exe + "\""; - const std::string cmd = exe + " " + args + " 2>&1"; - std::unique_ptr pipe(_popen(cmd.c_str(), "r"), _pclose); -#else - const std::string cmd = exe + " " + args + " 2>&1"; - std::unique_ptr pipe(popen(cmd.c_str(), "r"), pclose); -#endif - if (!pipe) - return false; - char buffer[1024]; - while (fgets(buffer, sizeof(buffer), pipe.get()) != nullptr) - *output += buffer; - return true; -} - -static std::string executeAddon(const AddonInfo &addonInfo, - const std::string &defaultPythonExe, - const std::string &dumpFile) -{ - std::string pythonExe; - - if (!addonInfo.python.empty()) - pythonExe = cmdFileName(addonInfo.python); - else if (!defaultPythonExe.empty()) - pythonExe = cmdFileName(defaultPythonExe); - else { -#ifdef _WIN32 - const char *p[] = { "python3.exe", "python.exe" }; -#else - const char *p[] = { "python3", "python" }; -#endif - for (int i = 0; i < 2; ++i) { - std::string out; - if (executeShellCommand(p[i], "--version", &out) && out.compare(0, 7, "Python ") == 0 && std::isdigit(out[7])) { - pythonExe = p[i]; - break; - } - } - if (pythonExe.empty()) - throw InternalError(nullptr, "Failed to auto detect python"); - } - - const std::string args = cmdFileName(addonInfo.scriptFile) + " --cli" + addonInfo.args + " " + cmdFileName(dumpFile); - std::string result; - if (!executeShellCommand(pythonExe, args, &result)) - throw InternalError(nullptr, "Failed to execute addon (command: '" + pythonExe + " " + args + "')"); - - // Validate output.. - std::istringstream istr(result); - std::string line; - while (std::getline(istr, line)) { - if (line.compare(0,9,"Checking ", 0, 9) != 0 && !line.empty() && line[0] != '{') - throw InternalError(nullptr, "Failed to execute '" + pythonExe + " " + args + "'. " + result); - } - - // Valid results - return result; -} - -static std::vector split(const std::string &str, const std::string &sep) +static std::vector split(const std::string &str, const std::string &sep=" ") { std::vector ret; for (std::string::size_type startPos = 0U; startPos < str.size();) { @@ -252,26 +186,63 @@ static std::vector split(const std::string &str, const std::string return ret; } -static std::pair executeCommand(const std::string &cmd) +static std::string executeAddon(const AddonInfo &addonInfo, + const std::string &defaultPythonExe, + const std::string &dumpFile, + std::function,std::string,std::string*)> executeCommand) { + const std::string redirect = "2>&1"; + + std::string pythonExe; + + if (!addonInfo.python.empty()) + pythonExe = cmdFileName(addonInfo.python); + else if (!defaultPythonExe.empty()) + pythonExe = cmdFileName(defaultPythonExe); + else { #ifdef _WIN32 - std::unique_ptr pipe(_popen(cmd.c_str(), "r"), _pclose); + const char *p[] = { "python3.exe", "python.exe" }; #else - std::unique_ptr pipe(popen(cmd.c_str(), "r"), pclose); + const char *p[] = { "python3", "python" }; #endif + for (int i = 0; i < 2; ++i) { + std::string out; + if (executeCommand(p[i], split("--version"), redirect, &out) && out.compare(0, 7, "Python ") == 0 && std::isdigit(out[7])) { + pythonExe = p[i]; + break; + } + } + if (pythonExe.empty()) + throw InternalError(nullptr, "Failed to auto detect python"); + } - if (!pipe) - return std::pair(false, ""); - - char buffer[1024]; + const std::string args = cmdFileName(addonInfo.scriptFile) + " --cli" + addonInfo.args + " " + cmdFileName(dumpFile); std::string result; - while (fgets(buffer, sizeof(buffer), pipe.get()) != nullptr) - result += buffer; - return std::pair(true, result); + if (!executeCommand(pythonExe, split(args), redirect, &result)) + throw InternalError(nullptr, "Failed to execute addon (command: '" + pythonExe + " " + args + "')"); + + // Validate output.. + std::istringstream istr(result); + std::string line; + while (std::getline(istr, line)) { + if (line.compare(0,9,"Checking ", 0, 9) != 0 && !line.empty() && line[0] != '{') + throw InternalError(nullptr, "Failed to execute '" + pythonExe + " " + args + "'. " + result); + } + + // Valid results + return result; } -CppCheck::CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions) - : mErrorLogger(errorLogger), mExitCode(0), mSuppressInternalErrorFound(false), mUseGlobalSuppressions(useGlobalSuppressions), mTooManyConfigs(false), mSimplify(true) +CppCheck::CppCheck(ErrorLogger &errorLogger, + bool useGlobalSuppressions, + std::function,std::string,std::string*)> executeCommand) + : mErrorLogger(errorLogger) + , mExitCode(0) + , mSuppressInternalErrorFound(false) + , mUseGlobalSuppressions(useGlobalSuppressions) + , mTooManyConfigs(false) + , mSimplify(true) + , mExecuteCommand(executeCommand) { } @@ -349,14 +320,18 @@ unsigned int CppCheck::check(const std::string &path) const std::string analyzerInfo = mSettings.buildDir.empty() ? std::string() : AnalyzerInformation::getAnalyzerInfoFile(mSettings.buildDir, path, ""); const std::string clangcmd = analyzerInfo + ".clang-cmd"; const std::string clangStderr = analyzerInfo + ".clang-stderr"; - - const std::string cmd1 = "clang -v -fsyntax-only " + lang + " " + tempFile + " 2>&1"; - const std::pair &result1 = executeCommand(cmd1); - if (!result1.first || result1.second.find(" -cc1 ") == std::string::npos) { - mErrorLogger.reportOut("Failed to execute '" + cmd1 + "':" + result1.second); +#ifdef _WIN32 + const std::string exe = "clang.exe"; +#else + const std::string exe = "clang"; +#endif + const std::string args1 = "-v -fsyntax-only " + lang + " " + tempFile; + std::string output1; + if (!mExecuteCommand(exe, split(args1), "2>&1", &output1) || output1.find(" -cc1 ") == std::string::npos) { + mErrorLogger.reportOut("Failed to execute '" + exe + "':" + output1); return 0; } - std::istringstream details(result1.second); + std::istringstream details(output1); std::string line; std::string flags(lang + " "); while (std::getline(details, line)) { @@ -374,15 +349,16 @@ unsigned int CppCheck::check(const std::string &path) for (const std::string &i: mSettings.includePaths) flags += "-I" + i + " "; - const std::string cmd = "clang -cc1 -ast-dump " + flags + path + (analyzerInfo.empty() ? std::string(" 2>&1") : (" 2> " + clangStderr)); + const std::string args2 = "-cc1 -ast-dump " + flags + path; + const std::string redirect2 = analyzerInfo.empty() ? std::string("2>&1") : ("2> " + clangStderr); if (!mSettings.buildDir.empty()) { std::ofstream fout(clangcmd); - fout << cmd << std::endl; + fout << exe << " " << args2 << " " << redirect2 << std::endl; } - const std::pair &result2 = executeCommand(cmd); - if (!result2.first || result2.second.find("TranslationUnitDecl") == std::string::npos) { - std::cerr << "Failed to execute '" + cmd + "'" << std::endl; + std::string output2; + if (!mExecuteCommand(exe,split(args2),redirect2,&output2) || output2.find("TranslationUnitDecl") == std::string::npos) { + std::cerr << "Failed to execute '" << exe << " " << args2 << " " << redirect2 << "'" << std::endl; return 0; } @@ -395,7 +371,7 @@ unsigned int CppCheck::check(const std::string &path) if (reportClangErrors(fin, reportError)) return 0; } else { - std::istringstream istr(result2.second); + std::istringstream istr(output2); auto reportError = [this](const ErrorLogger::ErrorMessage& errorMessage) { reportErr(errorMessage); }; @@ -404,7 +380,7 @@ unsigned int CppCheck::check(const std::string &path) } //std::cout << "Checking Clang ast dump:\n" << result2.second << std::endl; - std::istringstream ast(result2.second); + std::istringstream ast(output2); Tokenizer tokenizer(&mSettings, this); tokenizer.list.appendFileIfNew(path); clangimport::parseClangAstDump(&tokenizer, ast); @@ -427,7 +403,7 @@ unsigned int CppCheck::check(const std::string &path, const std::string &content unsigned int CppCheck::check(const ImportProject::FileSettings &fs) { - CppCheck temp(mErrorLogger, mUseGlobalSuppressions); + CppCheck temp(mErrorLogger, mUseGlobalSuppressions, mExecuteCommand); temp.mSettings = mSettings; if (!temp.mSettings.userDefines.empty()) temp.mSettings.userDefines += ';'; @@ -849,7 +825,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string continue; } const std::string results = - executeAddon(addonInfo, mSettings.addonPython, dumpFile); + executeAddon(addonInfo, mSettings.addonPython, dumpFile, mExecuteCommand); std::istringstream istr(results); std::string line; @@ -1441,15 +1417,21 @@ void CppCheck::analyseClangTidy(const ImportProject::FileSettings &fileSettings) pos += 3; } - const std::string cmd = "clang-tidy -quiet -checks=*,-clang-analyzer-*,-llvm* \"" + fileSettings.filename + "\" -- " + allIncludes + allDefines; - std::pair result = executeCommand(cmd); - if (!result.first) { - std::cerr << "Failed to execute '" + cmd + "'" << std::endl; +#ifdef _WIN32 + const char exe[] = "clang-tidy.exe"; +#else + const char exe[] = "clang-tidy"; +#endif + + const std::string args = "-quiet -checks=*,-clang-analyzer-*,-llvm* \"" + fileSettings.filename + "\" -- " + allIncludes + allDefines; + std::string output; + if (!mExecuteCommand(exe, split(args), "", &output)) { + std::cerr << "Failed to execute '" << exe << "'" << std::endl; return; } // parse output and create error messages - std::istringstream istr(result.second); + std::istringstream istr(output); std::string line; if (!mSettings.buildDir.empty()) { diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 4253cff90..ecab05713 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -50,7 +50,9 @@ public: /** * @brief Constructor. */ - CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions); + CppCheck(ErrorLogger &errorLogger, + bool useGlobalSuppressions, + std::function,std::string,std::string*)> executeCommand); /** * @brief Destructor. @@ -231,6 +233,9 @@ private: std::list mFileInfo; AnalyzerInformation mAnalyzerInformation; + + /** Callback for executing a shell command (exe, args, output) */ + std::function,std::string,std::string*)> mExecuteCommand; }; /// @} diff --git a/test/testcppcheck.cpp b/test/testcppcheck.cpp index dfe975849..c34b8c173 100644 --- a/test/testcppcheck.cpp +++ b/test/testcppcheck.cpp @@ -75,7 +75,7 @@ private: void getErrorMessages() const { ErrorLogger2 errorLogger; - CppCheck cppCheck(errorLogger, true); + CppCheck cppCheck(errorLogger, true, nullptr); cppCheck.getErrorMessages(); ASSERT(!errorLogger.id.empty()); diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 2dc6d24fb..4979c22a1 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -179,7 +179,7 @@ private: // Clear the error log errout.str(""); - CppCheck cppCheck(*this, true); + CppCheck cppCheck(*this, true, nullptr); Settings& settings = cppCheck.settings(); settings.exitCode = 1; settings.inlineSuppressions = true; @@ -630,7 +630,7 @@ private: void globalSuppressions() { // Testing that Cppcheck::useGlobalSuppressions works (#8515) errout.str(""); - CppCheck cppCheck(*this, false); // <- do not "use global suppressions". pretend this is a thread that just checks a file. + CppCheck cppCheck(*this, false, nullptr); // <- do not "use global suppressions". pretend this is a thread that just checks a file. Settings& settings = cppCheck.settings(); settings.nomsg.addSuppressionLine("uninitvar"); settings.exitCode = 1; @@ -662,7 +662,7 @@ private: // Clear the error log errout.str(""); - CppCheck cppCheck(*this, true); + CppCheck cppCheck(*this, true, nullptr); Settings& settings = cppCheck.settings(); settings.addEnabled("style"); settings.inlineSuppressions = true;