moved settings ownership out of CppCheck

danmar · Feb 5, 2025 · 598483e · 598483e
1 parent a6b7454
commit 598483e
Show file tree

Hide file tree

Showing 13 changed files with 129 additions and 111 deletions.
diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp
@@ -440,8 +440,7 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
     if (!settings.checkersReportFilename.empty())
         std::remove(settings.checkersReportFilename.c_str());
 
-    CppCheck cppcheck(supprs, stdLogger, true, executeCommand);
-    cppcheck.settings() = settings; // this is a copy
+    CppCheck cppcheck(settings, supprs, stdLogger, true, executeCommand);
 
     unsigned int returnValue = 0;
     if (settings.useSingleJob()) {

diff --git a/cli/processexecutor.cpp b/cli/processexecutor.cpp
@@ -284,13 +284,12 @@ unsigned int ProcessExecutor::check()
                 close(pipes[0]);
 
                 PipeWriter pipewriter(pipes[1]);
-                CppCheck fileChecker(mSuppressions, pipewriter, false, mExecuteCommand);
-                fileChecker.settings() = mSettings;
+                CppCheck fileChecker(mSettings, mSuppressions, pipewriter, false, mExecuteCommand);
                 unsigned int resultOfCheck = 0;
 
                 if (iFileSettings != mFileSettings.end()) {
                     resultOfCheck = fileChecker.check(*iFileSettings);
-                    if (fileChecker.settings().clangTidy)
+                    if (mSettings.clangTidy)
                         fileChecker.analyseClangTidy(*iFileSettings);
                 } else {
                     // Read file from a file

diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp
@@ -112,14 +112,13 @@ class ThreadData
     }
 
     unsigned int check(ErrorLogger &errorLogger, const FileWithDetails *file, const FileSettings *fs) const {
-        CppCheck fileChecker(mSuppressions, errorLogger, false, mExecuteCommand);
-        fileChecker.settings() = mSettings; // this is a copy
+        CppCheck fileChecker(mSettings, mSuppressions, errorLogger, false, mExecuteCommand);
 
         unsigned int result;
         if (fs) {
             // file settings..
             result = fileChecker.check(*fs);
-            if (fileChecker.settings().clangTidy)
+            if (mSettings.clangTidy)
                 fileChecker.analyseClangTidy(*fs);
         } else {
             // Read file from a file

diff --git a/democlient/democlient.cpp b/democlient/democlient.cpp
@@ -61,12 +61,9 @@ class CppcheckExecutor : public ErrorLogger {
     CppCheck cppcheck;
 
 public:
-    CppcheckExecutor()
-        : ErrorLogger()
-        , stoptime(std::time(nullptr)+2U)
-        , cppcheck(supprs, *this, false, nullptr) {
-        cppcheck.settings().addEnabled("all");
-        cppcheck.settings().certainty.enable(Certainty::inconclusive);
+    CppcheckExecutor(const Settings& settings)
+        : stoptime(std::time(nullptr)+2U)
+        , cppcheck(settings, supprs, *this, false, nullptr) {
     }
 
     void run(const char code[]) {
@@ -129,7 +126,10 @@ int main()
 
     std::cout << "<html><body>Cppcheck " CPPCHECK_VERSION_STRING "<pre>";
 
-    CppcheckExecutor cppcheckExecutor;
+    Settings s;
+    s.addEnabled("all");
+    s.certainty.enable(Certainty::inconclusive);
+    CppcheckExecutor cppcheckExecutor(s);
     cppcheckExecutor.run(code);
 
     std::fclose(logfile);

diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp
@@ -131,8 +131,7 @@ void CheckThread::run()
 {
     mState = Running;
 
-    CppCheck cppcheck(*mSuppressions, mResult, true, executeCommand);
-    cppcheck.settings() = std::move(mSettings);
+    CppCheck cppcheck(mSettings, *mSuppressions, mResult, true, executeCommand);
 
     if (!mFiles.isEmpty() || mAnalyseWholeProgram) {
         mAnalyseWholeProgram = false;
@@ -141,9 +140,9 @@ void CheckThread::run()
         qDebug() << "Whole program analysis";
         std::list<FileWithDetails> files2;
         std::transform(mFiles.cbegin(), mFiles.cend(), std::back_inserter(files2), [&](const QString& file) {
-            return FileWithDetails{file.toStdString(), Path::identify(file.toStdString(), cppcheck.settings().cppHeaderProbe), 0};
+            return FileWithDetails{file.toStdString(), Path::identify(file.toStdString(), mSettings.cppHeaderProbe), 0};
         });
-        cppcheck.analyseWholeProgram(cppcheck.settings().buildDir, files2, {}, ctuInfo);
+        cppcheck.analyseWholeProgram(mSettings.buildDir, files2, {}, ctuInfo);
         mFiles.clear();
         emit done();
         return;
@@ -153,7 +152,7 @@ void CheckThread::run()
     while (!file.isEmpty() && mState == Running) {
         qDebug() << "Checking file" << file;
         cppcheck.check(FileWithDetails(file.toStdString()));
-        runAddonsAndTools(cppcheck.settings(), nullptr, file);
+        runAddonsAndTools(mSettings, nullptr, file);
         emit fileChecked(file);
 
         if (mState == Running)
@@ -166,7 +165,7 @@ void CheckThread::run()
         file = QString::fromStdString(fileSettings->filename());
         qDebug() << "Checking file" << file;
         cppcheck.check(*fileSettings);
-        runAddonsAndTools(cppcheck.settings(), fileSettings, QString::fromStdString(fileSettings->filename()));
+        runAddonsAndTools(mSettings, fileSettings, QString::fromStdString(fileSettings->filename()));
         emit fileChecked(file);
 
         if (mState == Running)

diff --git a/gui/checkthread.h b/gui/checkthread.h
@@ -131,7 +131,7 @@ class CheckThread : public QThread {
 
     ThreadResult &mResult;
 
-    Settings mSettings;
+    Settings mSettings; // TODO: make ref
     Suppressions* mSuppressions;
 
 private:

diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp
@@ -695,8 +695,7 @@ void MainWindow::analyzeCode(const QString& code, const QString& filename)
             mUI->mResults, SLOT(debugError(ErrorItem)));
 
     // Create CppCheck instance
-    CppCheck cppcheck(supprs, result, true, nullptr);
-    cppcheck.settings() = checkSettings;
+    CppCheck cppcheck(checkSettings, supprs, result, true, nullptr);
 
     // Check
     checkLockDownUI();

diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp
@@ -32,6 +32,7 @@
 #include "path.h"
 #include "platform.h"
 #include "preprocessor.h"
+#include "settings.h"
 #include "standards.h"
 #include "suppressions.h"
 #include "timer.h"
@@ -528,11 +529,13 @@ static std::string getDefinesFlags(const std::string &semicolonSeparatedString)
     return flags;
 }
 
-CppCheck::CppCheck(Suppressions& supprs,
+CppCheck::CppCheck(const Settings& settings,
+                   Suppressions& supprs,
                    ErrorLogger &errorLogger,
                    bool useGlobalSuppressions,
                    ExecuteCmdFn executeCommand)
-    : mSuppressions(supprs)
+    : mSettings(settings)
+    , mSuppressions(supprs)
     , mLogger(new CppCheckLogger(errorLogger, mSettings, mSuppressions, useGlobalSuppressions))
     , mErrorLogger(*mLogger)
     , mErrorLoggerDirect(errorLogger)
@@ -783,31 +786,33 @@ unsigned int CppCheck::check(const FileSettings &fs)
     if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
         mUnusedFunctionsCheck.reset(new CheckUnusedFunctions());
 
-    // need to pass the externally provided ErrorLogger instead of our internal wrapper
-    CppCheck temp(mSuppressions, mErrorLoggerDirect, mUseGlobalSuppressions, mExecuteCommand);
-    temp.mSettings = mSettings;
-    if (!temp.mSettings.userDefines.empty())
-        temp.mSettings.userDefines += ';';
+    Settings tempSettings = mSettings; // this is a copy
+    if (!tempSettings.userDefines.empty())
+        tempSettings.userDefines += ';';
     if (mSettings.clang)
-        temp.mSettings.userDefines += fs.defines;
+        tempSettings.userDefines += fs.defines;
     else
-        temp.mSettings.userDefines += fs.cppcheckDefines();
-    temp.mSettings.includePaths = fs.includePaths;
-    temp.mSettings.userUndefs.insert(fs.undefs.cbegin(), fs.undefs.cend());
+        tempSettings.userDefines += fs.cppcheckDefines();
+    tempSettings.includePaths = fs.includePaths;
+    tempSettings.userUndefs.insert(fs.undefs.cbegin(), fs.undefs.cend());
     if (fs.standard.find("++") != std::string::npos)
-        temp.mSettings.standards.setCPP(fs.standard);
+        tempSettings.standards.setCPP(fs.standard);
     else if (!fs.standard.empty())
-        temp.mSettings.standards.setC(fs.standard);
+        tempSettings.standards.setC(fs.standard);
     if (fs.platformType != Platform::Type::Unspecified)
-        temp.mSettings.platform.set(fs.platformType);
+        tempSettings.platform.set(fs.platformType);
     if (mSettings.clang) {
-        temp.mSettings.includePaths.insert(temp.mSettings.includePaths.end(), fs.systemIncludePaths.cbegin(), fs.systemIncludePaths.cend());
+        tempSettings.includePaths.insert(tempSettings.includePaths.end(), fs.systemIncludePaths.cbegin(), fs.systemIncludePaths.cend());
+        // need to pass the externally provided ErrorLogger instead of our internal wrapper
+        CppCheck temp(tempSettings, mSuppressions, mErrorLoggerDirect, mUseGlobalSuppressions, mExecuteCommand);
         // TODO: propagate back mFileInfo
         const unsigned int returnValue = temp.check(fs.file);
         if (mUnusedFunctionsCheck)
             mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
         return returnValue;
     }
+    // need to pass the externally provided ErrorLogger instead of our internal wrapper
+    CppCheck temp(tempSettings, mSuppressions, mErrorLoggerDirect, mUseGlobalSuppressions, mExecuteCommand);
     const unsigned int returnValue = temp.checkFile(fs.file, fs.cfg);
     if (mUnusedFunctionsCheck)
         mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
@@ -1791,11 +1796,6 @@ void CppCheck::executeAddonsWholeProgram(const std::list<FileWithDetails> &files
     executeAddons(ctuInfoFiles, "");
 }
 
-Settings &CppCheck::settings()
-{
-    return mSettings;
-}
-
 void CppCheck::tooManyConfigsError(const std::string &file, const int numberOfConfigurations)
 {
     if (!mSettings.severity.isEnabled(Severity::information) && !mTooManyConfigs)
@@ -1861,16 +1861,18 @@ void CppCheck::purgedConfigurationMessage(const std::string &file, const std::st
 
 void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
 {
-    Settings s;
-    s.addEnabled("all");
+    Settings settings;
     Suppressions supprs;
 
-    CppCheck cppcheck(supprs, errorlogger, true, nullptr);
+    CppCheck cppcheck(settings, supprs, errorlogger, true, nullptr);
     cppcheck.purgedConfigurationMessage("","");
     cppcheck.mTooManyConfigs = true;
     cppcheck.tooManyConfigsError("",0U);
     // TODO: add functions to get remaining error messages
 
+    Settings s;
+    s.addEnabled("all");
+
     // call all "getErrorMessages" in all registered Check classes
     for (auto it = Check::instances().cbegin(); it != Check::instances().cend(); ++it)
         (*it)->getErrorMessages(&errorlogger, &s);

diff --git a/lib/cppcheck.h b/lib/cppcheck.h
@@ -23,7 +23,6 @@
 
 #include "check.h"
 #include "config.h"
-#include "settings.h"
 #include "standards.h"
 
 #include <cstdint>
@@ -42,6 +41,8 @@ class Tokenizer;
 class FileWithDetails;
 class AnalyzerInformation;
 class ErrorLogger;
+class Settings;
+struct Suppressions;
 
 namespace simplecpp { class TokenList; }
 
@@ -61,7 +62,8 @@ class CPPCHECKLIB CppCheck {
     /**
      * @brief Constructor.
      */
-    CppCheck(Suppressions& supprs,
+    CppCheck(const Settings& settings,
+             Suppressions& supprs,
              ErrorLogger &errorLogger,
              bool useGlobalSuppressions,
              ExecuteCmdFn executeCommand);
@@ -101,12 +103,6 @@ class CPPCHECKLIB CppCheck {
      */
     unsigned int check(const FileWithDetails &file, const std::string &content);
 
-    /**
-     * @brief Get reference to current settings.
-     * @return a reference to current settings
-     */
-    Settings &settings();
-
     /**
      * @brief Returns current version number as a string.
      * @return version, e.g. "1.38"
@@ -209,7 +205,7 @@ class CPPCHECKLIB CppCheck {
 
     unsigned int checkClang(const FileWithDetails &file);
 
-    Settings mSettings;
+    const Settings& mSettings;
     Suppressions& mSuppressions;
 
     class CppCheckLogger;

diff --git a/oss-fuzz/main.cpp b/oss-fuzz/main.cpp
@@ -50,10 +50,12 @@ static const FileWithDetails s_file("test.cpp");
 
 static void doCheck(const std::string& code)
 {
+    // TODO: create the settings only once
+    Settings s;
+    s.addEnabled("all");
+    s.certainty.setEnabled(Certainty::inconclusive, true);
     Suppressions supprs;
-    CppCheck cppcheck(supprs, s_errorLogger, false, nullptr);
-    cppcheck.settings().addEnabled("all");
-    cppcheck.settings().certainty.setEnabled(Certainty::inconclusive, true);
+    CppCheck cppcheck(s, supprs, s_errorLogger, false, nullptr);
     cppcheck.check(s_file, code);
 }