executeCommand in CppCheckExecutor or QCheckThread

danmar · danmar · commit 32e569704bd1 · 2020-05-19T16:04:40.000+02:00
diff --git 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
@@ -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<std::string> 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<FILE, decltype(&_pclose)> pipe(_popen(cmd.c_str(), "r"), _pclose);
+#else
+    const std::string cmd = exe + " " + joinedArgs + " " + redirect;
+    std::unique_ptr<FILE, decltype(&pclose)> 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
@@ -108,6 +108,11 @@ class CppCheckExecutor : public ErrorLogger {
     */
     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<std::string> args, std::string redirect, std::string *output);
+
 protected:
 
     /**
diff --git 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
@@ -32,10 +32,35 @@
 #include "cppcheck.h"
 #include "common.h"
 
+static bool executeCommand(std::string exe, std::vector<std::string> 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
@@ -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
@@ -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
@@ -164,32 +164,35 @@ static std::string cmdFileName(std::string f)
     return f;
 }
 
-static bool executeShellCommand(std::string exe, const std::string &args, std::string *output)
+static std::vector<std::string> split(const std::string &str, const std::string &sep=" ")
 {
-    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<FILE, decltype(&_pclose)> pipe(_popen(cmd.c_str(), "r"), _pclose);
-#else
-    const std::string cmd = exe + " " + args + " 2>&1";
-    std::unique_ptr<FILE, decltype(&pclose)> 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;
+    std::vector<std::string> ret;
+    for (std::string::size_type startPos = 0U; startPos < str.size();) {
+        std::string::size_type endPos;
+        if (str[startPos] == '\"') {
+            endPos = str.find("\"", startPos + 1);
+            if (endPos < str.size())
+                endPos++;
+        } else {
+            endPos = str.find(sep, startPos + 1);
+        }
+        if (endPos == std::string::npos) {
+            ret.push_back(str.substr(startPos));
+            break;
+        }
+        ret.push_back(str.substr(startPos, endPos - startPos));
+        startPos = str.find_first_not_of(sep, endPos);
+    }
+    return ret;
 }
 
 static std::string executeAddon(const AddonInfo &addonInfo,
                                 const std::string &defaultPythonExe,
-                                const std::string &dumpFile)
+                                const std::string &dumpFile,
+                                std::function<bool(std::string,std::vector<std::string>,std::string,std::string*)> executeCommand)
 {
+    const std::string redirect = "2>&1";
+
     std::string pythonExe;
 
     if (!addonInfo.python.empty())
@@ -204,7 +207,7 @@ static std::string executeAddon(const AddonInfo &addonInfo,
 #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])) {
+            if (executeCommand(p[i], split("--version"), redirect, &out) && out.compare(0, 7, "Python ") == 0 && std::isdigit(out[7])) {
                 pythonExe = p[i];
                 break;
             }
@@ -215,7 +218,7 @@ static std::string executeAddon(const AddonInfo &addonInfo,
 
     const std::string args = cmdFileName(addonInfo.scriptFile) + " --cli" + addonInfo.args + " " + cmdFileName(dumpFile);
     std::string result;
-    if (!executeShellCommand(pythonExe, args, &result))
+    if (!executeCommand(pythonExe, split(args), redirect, &result))
         throw InternalError(nullptr, "Failed to execute addon (command: '" + pythonExe + " " + args + "')");
 
     // Validate output..
@@ -230,48 +233,16 @@ static std::string executeAddon(const AddonInfo &addonInfo,
     return result;
 }
 
-static std::vector<std::string> split(const std::string &str, const std::string &sep)
-{
-    std::vector<std::string> ret;
-    for (std::string::size_type startPos = 0U; startPos < str.size();) {
-        std::string::size_type endPos;
-        if (str[startPos] == '\"') {
-            endPos = str.find("\"", startPos + 1);
-            if (endPos < str.size())
-                endPos++;
-        } else {
-            endPos = str.find(sep, startPos + 1);
-        }
-        if (endPos == std::string::npos) {
-            ret.push_back(str.substr(startPos));
-            break;
-        }
-        ret.push_back(str.substr(startPos, endPos - startPos));
-        startPos = str.find_first_not_of(sep, endPos);
-    }
-    return ret;
-}
-
-static std::pair<bool,std::string> executeCommand(const std::string &cmd)
-{
-#ifdef _WIN32
-    std::unique_ptr<FILE, decltype(&_pclose)> pipe(_popen(cmd.c_str(), "r"), _pclose);
-#else
-    std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd.c_str(), "r"), pclose);
-#endif
-
-    if (!pipe)
-        return std::pair<bool, std::string>(false, "");
-
-    char buffer[1024];
-    std::string result;
-    while (fgets(buffer, sizeof(buffer), pipe.get()) != nullptr)
-        result += buffer;
-    return std::pair<bool, std::string>(true, 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<bool(std::string,std::vector<std::string>,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<bool, std::string> &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<bool, std::string> &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<bool, std::string> 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
@@ -50,7 +50,9 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
     /**
      * @brief Constructor.
      */
-    CppCheck(ErrorLogger &errorLogger, bool useGlobalSuppressions);
+    CppCheck(ErrorLogger &errorLogger,
+             bool useGlobalSuppressions,
+             std::function<bool(std::string,std::vector<std::string>,std::string,std::string*)> executeCommand);
 
     /**
      * @brief Destructor.
@@ -231,6 +233,9 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
     std::list<Check::FileInfo*> mFileInfo;
 
     AnalyzerInformation mAnalyzerInformation;
+
+    /** Callback for executing a shell command (exe, args, output) */
+    std::function<bool(std::string,std::vector<std::string>,std::string,std::string*)> mExecuteCommand;
 };
 
 /// @}
diff --git a/test/testcppcheck.cpp b/test/testcppcheck.cpp
diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp
