some CppCheck and related usage cleanups (#5979)

Author: firewave

cli/cppcheckexecutor.cpp

@@ -64,109 +64,111 @@
 #include <windows.h>
 #endif
 
-class CmdLineLoggerStd : public CmdLineLogger
-{
-public:
-    CmdLineLoggerStd() = default;
-
-    void printMessage(const std::string &message) override
+namespace {
+    class CmdLineLoggerStd : public CmdLineLogger
     {
-        printRaw("cppcheck: " + message);
-    }
+    public:
+        CmdLineLoggerStd() = default;
 
-    void printError(const std::string &message) override
-    {
-        printMessage("error: " + message);
-    }
+        void printMessage(const std::string &message) override
+        {
+            printRaw("cppcheck: " + message);
+        }
 
-    void printRaw(const std::string &message) override
-    {
-        std::cout << message << std::endl;
-    }
-};
+        void printError(const std::string &message) override
+        {
+            printMessage("error: " + message);
+        }
 
-class CppCheckExecutor::StdLogger : public ErrorLogger
-{
-public:
-    explicit StdLogger(const Settings& settings)
-        : mSettings(settings)
+        void printRaw(const std::string &message) override
+        {
+            std::cout << message << std::endl;
+        }
+    };
+
+    class StdLogger : public ErrorLogger
     {
-        if (!mSettings.outputFile.empty()) {
-            mErrorOutput = new std::ofstream(settings.outputFile);
+    public:
+        explicit StdLogger(const Settings& settings)
+            : mSettings(settings)
+        {
+            if (!mSettings.outputFile.empty()) {
+                mErrorOutput = new std::ofstream(settings.outputFile);
+            }
         }
-    }
 
-    ~StdLogger() override {
-        delete mErrorOutput;
-    }
+        ~StdLogger() override {
+            delete mErrorOutput;
+        }
 
-    StdLogger(const StdLogger&) = delete;
-    StdLogger& operator=(const SingleExecutor &) = delete;
+        StdLogger(const StdLogger&) = delete;
+        StdLogger& operator=(const SingleExecutor &) = delete;
 
-    void resetLatestProgressOutputTime() {
-        mLatestProgressOutputTime = std::time(nullptr);
-    }
+        void resetLatestProgressOutputTime() {
+            mLatestProgressOutputTime = std::time(nullptr);
+        }
 
-    /**
-     * Helper function to print out errors. Appends a line change.
-     * @param errmsg String printed to error stream
-     */
-    void reportErr(const std::string &errmsg);
+        /**
+         * Helper function to print out errors. Appends a line change.
+         * @param errmsg String printed to error stream
+         */
+        void reportErr(const std::string &errmsg);
 
-    /**
-     * @brief Write the checkers report
-     */
-    void writeCheckersReport();
+        /**
+         * @brief Write the checkers report
+         */
+        void writeCheckersReport();
 
-    bool hasCriticalErrors() const {
-        return !mCriticalErrors.empty();
-    }
+        bool hasCriticalErrors() const {
+            return !mCriticalErrors.empty();
+        }
 
-private:
-    /**
-     * Information about progress is directed here. This should be
-     * called by the CppCheck class only.
-     *
-     * @param outmsg Progress message e.g. "Checking main.cpp..."
-     */
-    void reportOut(const std::string &outmsg, Color c = Color::Reset) override;
-
-    /** xml output of errors */
-    void reportErr(const ErrorMessage &msg) override;
-
-    void reportProgress(const std::string &filename, const char stage[], const std::size_t value) override;
-
-    /**
-     * Pointer to current settings; set while check() is running for reportError().
-     */
-    const Settings& mSettings;
-
-    /**
-     * Used to filter out duplicate error messages.
-     */
-    // TODO: store hashes instead of the full messages
-    std::unordered_set<std::string> mShownErrors;
-
-    /**
-     * Report progress time
-     */
-    std::time_t mLatestProgressOutputTime{};
-
-    /**
-     * Error output
-     */
-    std::ofstream* mErrorOutput{};
-
-    /**
-     * Checkers that has been executed
-     */
-    std::set<std::string> mActiveCheckers;
-
-    /**
-     * True if there are critical errors
-     */
-    std::string mCriticalErrors;
-};
+    private:
+        /**
+         * Information about progress is directed here. This should be
+         * called by the CppCheck class only.
+         *
+         * @param outmsg Progress message e.g. "Checking main.cpp..."
+         */
+        void reportOut(const std::string &outmsg, Color c = Color::Reset) override;
+
+        /** xml output of errors */
+        void reportErr(const ErrorMessage &msg) override;
+
+        void reportProgress(const std::string &filename, const char stage[], const std::size_t value) override;
+
+        /**
+         * Pointer to current settings; set while check() is running for reportError().
+         */
+        const Settings& mSettings;
+
+        /**
+         * Used to filter out duplicate error messages.
+         */
+        // TODO: store hashes instead of the full messages
+        std::unordered_set<std::string> mShownErrors;
+
+        /**
+         * Report progress time
+         */
+        std::time_t mLatestProgressOutputTime{};
+
+        /**
+         * Error output
+         */
+        std::ofstream* mErrorOutput{};
+
+        /**
+         * Checkers that has been executed
+         */
+        std::set<std::string> mActiveCheckers;
+
+        /**
+         * True if there are critical errors
+         */
+        std::string mCriticalErrors;
+    };
+}
 
 // TODO: do not directly write to stdout
 
@@ -193,28 +195,21 @@ int CppCheckExecutor::check(int argc, const char* const argv[])
 
     settings.setMisraRuleTexts(executeCommand);
 
-    mStdLogger = new StdLogger(settings);
-
-    CppCheck cppCheck(*mStdLogger, true, executeCommand);
-    cppCheck.settings() = settings; // this is a copy
-
-    const int ret = check_wrapper(cppCheck);
+    const int ret = check_wrapper(settings);
 
-    delete mStdLogger;
-    mStdLogger = nullptr;
     return ret;
 }
 
-int CppCheckExecutor::check_wrapper(CppCheck& cppcheck)
+int CppCheckExecutor::check_wrapper(const Settings& settings)
 {
 #ifdef USE_WINDOWS_SEH
-    if (cppcheck.settings().exceptionHandling)
-        return check_wrapper_seh(*this, &CppCheckExecutor::check_internal, cppcheck);
+    if (settings.exceptionHandling)
+        return check_wrapper_seh(*this, &CppCheckExecutor::check_internal, settings);
 #elif defined(USE_UNIX_SIGNAL_HANDLING)
-    if (cppcheck.settings().exceptionHandling)
-        return check_wrapper_sig(*this, &CppCheckExecutor::check_internal, cppcheck);
+    if (settings.exceptionHandling)
+        return check_wrapper_sig(*this, &CppCheckExecutor::check_internal, settings);
 #endif
-    return check_internal(cppcheck);
+    return check_internal(settings);
 }
 
 bool CppCheckExecutor::reportSuppressions(const Settings &settings, const Suppressions& suppressions, bool unusedFunctionCheckEnabled, const std::list<std::pair<std::string, std::size_t>> &files, const std::list<FileSettings>& fileSettings, ErrorLogger& errorLogger) {
@@ -246,16 +241,15 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, const Suppre
 /*
  * That is a method which gets called from check_wrapper
  * */
-int CppCheckExecutor::check_internal(CppCheck& cppcheck) const
+int CppCheckExecutor::check_internal(const Settings& settings) const
 {
-    const auto& settings = cppcheck.settings();
-    auto& suppressions = cppcheck.settings().nomsg;
+    StdLogger stdLogger(settings);
 
     if (settings.reportProgress >= 0)
-        mStdLogger->resetLatestProgressOutputTime();
+        stdLogger.resetLatestProgressOutputTime();
 
     if (settings.xml) {
-        mStdLogger->reportErr(ErrorMessage::getXMLHeader(settings.cppcheckCfgProductName));
+        stdLogger.reportErr(ErrorMessage::getXMLHeader(settings.cppcheckCfgProductName));
     }
 
     if (!settings.buildDir.empty()) {
@@ -268,24 +262,28 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const
     if (!settings.checkersReportFilename.empty())
         std::remove(settings.checkersReportFilename.c_str());
 
+    CppCheck cppcheck(stdLogger, true, executeCommand);
+    cppcheck.settings() = settings; // this is a copy
+    auto& suppressions = cppcheck.settings().nomsg;
+
     unsigned int returnValue;
     if (settings.useSingleJob()) {
         // Single process
-        SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, suppressions, *mStdLogger);
+        SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, suppressions, stdLogger);
         returnValue = executor.check();
     } else {
 #if defined(THREADING_MODEL_THREAD)
-        ThreadExecutor executor(mFiles, mFileSettings, settings, suppressions, *mStdLogger, CppCheckExecutor::executeCommand);
+        ThreadExecutor executor(mFiles, mFileSettings, settings, suppressions, stdLogger, CppCheckExecutor::executeCommand);
 #elif defined(THREADING_MODEL_FORK)
-        ProcessExecutor executor(mFiles, mFileSettings, settings, suppressions, *mStdLogger, CppCheckExecutor::executeCommand);
+        ProcessExecutor executor(mFiles, mFileSettings, settings, suppressions, stdLogger, CppCheckExecutor::executeCommand);
 #endif
         returnValue = executor.check();
     }
 
     cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings);
 
     if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) {
-        const bool err = reportSuppressions(settings, settings.nomsg, cppcheck.isUnusedFunctionCheckEnabled(), mFiles, mFileSettings, *mStdLogger);
+        const bool err = reportSuppressions(settings, suppressions, settings.isUnusedFunctionCheckEnabled(), mFiles, mFileSettings, stdLogger);
         if (err && returnValue == 0)
             returnValue = settings.exitCode;
     }
@@ -295,21 +293,21 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const
     }
 
     if (settings.safety || settings.severity.isEnabled(Severity::information) || !settings.checkersReportFilename.empty())
-        mStdLogger->writeCheckersReport();
+        stdLogger.writeCheckersReport();
 
     if (settings.xml) {
-        mStdLogger->reportErr(ErrorMessage::getXMLFooter());
+        stdLogger.reportErr(ErrorMessage::getXMLFooter());
     }
 
-    if (settings.safety && mStdLogger->hasCriticalErrors())
+    if (settings.safety && stdLogger.hasCriticalErrors())
         return EXIT_FAILURE;
 
     if (returnValue)
         return settings.exitCode;
     return EXIT_SUCCESS;
 }
 
-void CppCheckExecutor::StdLogger::writeCheckersReport()
+void StdLogger::writeCheckersReport()
 {
     CheckersReport checkersReport(mSettings, mActiveCheckers);
 
@@ -368,7 +366,7 @@ static inline std::string ansiToOEM(const std::string &msg, bool doConvert)
 #define ansiToOEM(msg, doConvert) (msg)
 #endif
 
-void CppCheckExecutor::StdLogger::reportErr(const std::string &errmsg)
+void StdLogger::reportErr(const std::string &errmsg)
 {
     if (mErrorOutput)
         *mErrorOutput << errmsg << std::endl;
@@ -377,7 +375,7 @@ void CppCheckExecutor::StdLogger::reportErr(const std::string &errmsg)
     }
 }
 
-void CppCheckExecutor::StdLogger::reportOut(const std::string &outmsg, Color c)
+void StdLogger::reportOut(const std::string &outmsg, Color c)
 {
     if (c == Color::Reset)
         std::cout << ansiToOEM(outmsg, true) << std::endl;
@@ -386,7 +384,7 @@ void CppCheckExecutor::StdLogger::reportOut(const std::string &outmsg, Color c)
 }
 
 // TODO: remove filename parameter?
-void CppCheckExecutor::StdLogger::reportProgress(const std::string &filename, const char stage[], const std::size_t value)
+void StdLogger::reportProgress(const std::string &filename, const char stage[], const std::size_t value)
 {
     (void)filename;
 
@@ -410,7 +408,7 @@ void CppCheckExecutor::StdLogger::reportProgress(const std::string &filename, co
     }
 }
 
-void CppCheckExecutor::StdLogger::reportErr(const ErrorMessage &msg)
+void StdLogger::reportErr(const ErrorMessage &msg)
 {
     if (msg.severity == Severity::internal && (msg.id == "logChecker" || endsWith(msg.id, "-logChecker"))) {
         const std::string& checker = msg.shortMessage();

cli/cppcheckexecutor.h

@@ -28,7 +28,6 @@
 #include <utility>
 #include <vector>
 
-class CppCheck;
 class Settings;
 class ErrorLogger;
 class Suppressions;
@@ -88,21 +87,21 @@ class CppCheckExecutor {
      * Wrapper around check_internal
      *   - installs optional platform dependent signal handling
      *
-     * @param cppcheck cppcheck instance
+     * @param settings the settings
      **/
-    int check_wrapper(CppCheck& cppcheck);
+    int check_wrapper(const Settings& settings);
 
     /**
      * Starts the checking.
      *
-     * @param cppcheck cppcheck instance
+     * @param settings the settings
      * @return EXIT_FAILURE if arguments are invalid or no input files
      *         were found.
      *         If errors are found and --error-exitcode is used,
      *         given value is returned instead of default 0.
      *         If no errors are found, 0 is returned.
      */
-    int check_internal(CppCheck& cppcheck) const;
+    int check_internal(const Settings& settings) const;
 
     /**
      * Filename associated with size of file
@@ -117,9 +116,6 @@ class CppCheckExecutor {
      */
     static FILE* mExceptionOutput;
 #endif
-
-    class StdLogger;
-    StdLogger* mStdLogger{};
 };
 
 #endif // CPPCHECKEXECUTOR_H