extracted code for actual check implementation from Check into CheckImpl

Author: firewave

Makefile

@@ -60,32 +60,6 @@ void Check::writeToErrorList(const ErrorMessage &errmsg)
     std::cout << errmsg.toXML() << std::endl;
 }
 
-
-void Check::reportError(const std::list<const Token *> &callstack, Severity severity, const std::string &id, const std::string &msg, const CWE &cwe, Certainty certainty)
-{
-    const ErrorMessage errmsg(callstack, mTokenizer ? &mTokenizer->list : nullptr, severity, id, msg, cwe, certainty);
-    if (mErrorLogger)
-        mErrorLogger->reportErr(errmsg);
-    else
-        writeToErrorList(errmsg);
-}
-
-void Check::reportError(const ErrorPath &errorPath, Severity severity, const char id[], const std::string &msg, const CWE &cwe, Certainty certainty)
-{
-    const ErrorMessage errmsg(errorPath, mTokenizer ? &mTokenizer->list : nullptr, severity, id, msg, cwe, certainty);
-    if (mErrorLogger)
-        mErrorLogger->reportErr(errmsg);
-    else
-        writeToErrorList(errmsg);
-}
-
-bool Check::wrongData(const Token *tok, const char *str)
-{
-    if (mSettings->daca)
-        reportError(tok, Severity::debug, "DacaWrongData", "Wrong data detected by condition " + std::string(str));
-    return true;
-}
-
 std::list<Check *> &Check::instances()
 {
 #ifdef __SVR4
@@ -98,36 +72,3 @@ std::list<Check *> &Check::instances()
     return _instances;
 #endif
 }
-
-std::string Check::getMessageId(const ValueFlow::Value &value, const char id[])
-{
-    if (value.condition != nullptr)
-        return id + std::string("Cond");
-    if (value.safe)
-        return std::string("safe") + (char)std::toupper(id[0]) + (id + 1);
-    return id;
-}
-
-ErrorPath Check::getErrorPath(const Token* errtok, const ValueFlow::Value* value, std::string bug) const
-{
-    ErrorPath errorPath;
-    if (!value) {
-        errorPath.emplace_back(errtok, std::move(bug));
-    } else if (mSettings->verbose || mSettings->xml || !mSettings->templateLocation.empty()) {
-        errorPath = value->errorPath;
-        errorPath.emplace_back(errtok, std::move(bug));
-    } else {
-        if (value->condition)
-            errorPath.emplace_back(value->condition, "condition '" + value->condition->expressionString() + "'");
-        //else if (!value->isKnown() || value->defaultArg)
-        //    errorPath = value->callstack;
-        errorPath.emplace_back(errtok, std::move(bug));
-    }
-    return errorPath;
-}
-
-void Check::logChecker(const char id[])
-{
-    reportError(nullptr, Severity::none, "logChecker", id);
-}
-

lib/check.cpp

@@ -61,15 +61,8 @@ class CPPCHECKLIB Check {
     /** This constructor is used when registering the CheckClass */
     explicit Check(const std::string &aname);
 
-protected:
-    /** This constructor is used when running checks. */
-    Check(std::string aname, const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
-        : mTokenizer(tokenizer), mSettings(settings), mErrorLogger(errorLogger), mName(std::move(aname)) {}
-
-public:
     virtual ~Check() {
-        if (!mTokenizer)
-            instances().remove(this);
+        instances().remove(this);
     }
 
     Check(const Check &) = delete;
@@ -129,45 +122,6 @@ class CPPCHECKLIB Check {
         return false;
     }
 
-protected:
-    static std::string getMessageId(const ValueFlow::Value &value, const char id[]);
-
-    const Tokenizer* const mTokenizer{};
-    const Settings* const mSettings{};
-    ErrorLogger* const mErrorLogger{};
-
-    /** report an error */
-    void reportError(const Token *tok, const Severity severity, const std::string &id, const std::string &msg) {
-        reportError(tok, severity, id, msg, CWE(0U), Certainty::normal);
-    }
-
-    /** report an error */
-    void reportError(const Token *tok, const Severity severity, const std::string &id, const std::string &msg, const CWE &cwe, Certainty certainty) {
-        const std::list<const Token *> callstack(1, tok);
-        reportError(callstack, severity, id, msg, cwe, certainty);
-    }
-
-    /** report an error */
-    void reportError(const std::list<const Token *> &callstack, Severity severity, const std::string &id, const std::string &msg) {
-        reportError(callstack, severity, id, msg, CWE(0U), Certainty::normal);
-    }
-
-    /** report an error */
-    void reportError(const std::list<const Token *> &callstack, Severity severity, const std::string &id, const std::string &msg, const CWE &cwe, Certainty certainty);
-
-    void reportError(const ErrorPath &errorPath, Severity severity, const char id[], const std::string &msg, const CWE &cwe, Certainty certainty);
-
-    /** log checker */
-    void logChecker(const char id[]);
-
-    ErrorPath getErrorPath(const Token* errtok, const ValueFlow::Value* value, std::string bug) const;
-
-    /**
-     * Use WRONG_DATA in checkers when you check for wrong data. That
-     * will call this method
-     */
-    bool wrongData(const Token *tok, const char *str);
-
 private:
     const std::string mName;
 };

lib/check.h

@@ -40,7 +40,7 @@ namespace {
     Check64BitPortability instance;
 }
 
-void Check64BitPortability::pointerassignment()
+void Check64BitPortabilityImpl::pointerassignment()
 {
     if (!mSettings->severity.isEnabled(Severity::portability))
         return;
@@ -118,7 +118,7 @@ void Check64BitPortability::pointerassignment()
     }
 }
 
-void Check64BitPortability::assignmentAddressToIntegerError(const Token *tok)
+void Check64BitPortabilityImpl::assignmentAddressToIntegerError(const Token *tok)
 {
     reportError(tok, Severity::portability,
                 "AssignmentAddressToInteger",
@@ -129,7 +129,7 @@ void Check64BitPortability::assignmentAddressToIntegerError(const Token *tok)
                 "way is to store addresses only in pointer types (or typedefs like uintptr_t).", CWE758, Certainty::normal);
 }
 
-void Check64BitPortability::assignmentIntegerToAddressError(const Token *tok)
+void Check64BitPortabilityImpl::assignmentIntegerToAddressError(const Token *tok)
 {
     reportError(tok, Severity::portability,
                 "AssignmentIntegerToAddress",
@@ -140,7 +140,7 @@ void Check64BitPortability::assignmentIntegerToAddressError(const Token *tok)
                 "way is to store addresses only in pointer types (or typedefs like uintptr_t).", CWE758, Certainty::normal);
 }
 
-void Check64BitPortability::returnPointerError(const Token *tok)
+void Check64BitPortabilityImpl::returnPointerError(const Token *tok)
 {
     reportError(tok, Severity::portability,
                 "CastAddressToIntegerAtReturn",
@@ -151,7 +151,7 @@ void Check64BitPortability::returnPointerError(const Token *tok)
                 "to 32-bit integer. The safe way is to always return an integer.", CWE758, Certainty::normal);
 }
 
-void Check64BitPortability::returnIntegerError(const Token *tok)
+void Check64BitPortabilityImpl::returnIntegerError(const Token *tok)
 {
     reportError(tok, Severity::portability,
                 "CastIntegerToAddressAtReturn",
@@ -161,3 +161,20 @@ void Check64BitPortability::returnIntegerError(const Token *tok)
                 "and Linux they are of different width. In worst case you end up casting 64-bit integer down to 32-bit pointer. "
                 "The safe way is to always return a pointer.", CWE758, Certainty::normal);
 }
+
+
+/** @brief Run checks against the normal token list */
+void Check64BitPortability::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
+{
+    Check64BitPortabilityImpl c(&tokenizer, tokenizer.getSettings(), errorLogger);
+    c.pointerassignment();
+}
+
+void Check64BitPortability::getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const
+{
+    Check64BitPortabilityImpl c(nullptr, settings, errorLogger);
+    c.assignmentAddressToIntegerError(nullptr);
+    c.assignmentIntegerToAddressError(nullptr);
+    c.returnIntegerError(nullptr);
+    c.returnPointerError(nullptr);
+}

lib/check64bit.cpp

@@ -23,6 +23,7 @@
 //---------------------------------------------------------------------------
 
 #include "check.h"
+#include "checkimpl.h"
 #include "config.h"
 #include "tokenize.h"
 
@@ -45,18 +46,26 @@ class CPPCHECKLIB Check64BitPortability : public Check {
 
 public:
     /** This constructor is used when registering the Check64BitPortability */
-    Check64BitPortability() : Check(myName()) {}
+    Check64BitPortability() : Check("64-bit portability") {}
 
 private:
-    /** This constructor is used when running checks. */
-    Check64BitPortability(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
-        : Check(myName(), tokenizer, settings, errorLogger) {}
-
     /** @brief Run checks against the normal token list */
-    void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override {
-        Check64BitPortability check64BitPortability(&tokenizer, tokenizer.getSettings(), errorLogger);
-        check64BitPortability.pointerassignment();
+    void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override;
+
+    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
+
+    std::string classInfo() const override {
+        return "Check if there is 64-bit portability issues:\n"
+               "- assign address to/from int/long\n"
+               "- casting address from/to integer when returning from function\n";
     }
+};
+
+class Check64BitPortabilityImpl : public CheckImpl {
+public:
+    /** This constructor is used when running checks. */
+    Check64BitPortabilityImpl(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
+        : CheckImpl(tokenizer, settings, errorLogger) {}
 
     /** Check for pointer assignment */
     void pointerassignment();
@@ -65,24 +74,6 @@ class CPPCHECKLIB Check64BitPortability : public Check {
     void assignmentIntegerToAddressError(const Token *tok);
     void returnIntegerError(const Token *tok);
     void returnPointerError(const Token *tok);
-
-    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {
-        Check64BitPortability c(nullptr, settings, errorLogger);
-        c.assignmentAddressToIntegerError(nullptr);
-        c.assignmentIntegerToAddressError(nullptr);
-        c.returnIntegerError(nullptr);
-        c.returnPointerError(nullptr);
-    }
-
-    static std::string myName() {
-        return "64-bit portability";
-    }
-
-    std::string classInfo() const override {
-        return "Check if there is 64-bit portability issues:\n"
-               "- assign address to/from int/long\n"
-               "- casting address from/to integer when returning from function\n";
-    }
 };
 /// @}
 //---------------------------------------------------------------------------

lib/check64bit.h

@@ -39,7 +39,7 @@ namespace {
     CheckAssert instance;
 }
 
-void CheckAssert::assertWithSideEffects()
+void CheckAssertImpl::assertWithSideEffects()
 {
     if (!mSettings->severity.isEnabled(Severity::warning))
         return;
@@ -99,7 +99,7 @@ void CheckAssert::assertWithSideEffects()
 //---------------------------------------------------------------------------
 
 
-void CheckAssert::sideEffectInAssertError(const Token *tok, const std::string& functionName)
+void CheckAssertImpl::sideEffectInAssertError(const Token *tok, const std::string& functionName)
 {
     reportError(tok, Severity::warning,
                 "assertWithSideEffect",
@@ -111,7 +111,7 @@ void CheckAssert::sideEffectInAssertError(const Token *tok, const std::string& f
                 "builds, this is a bug.", CWE398, Certainty::normal);
 }
 
-void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& varname)
+void CheckAssertImpl::assignmentInAssertError(const Token *tok, const std::string& varname)
 {
     reportError(tok, Severity::warning,
                 "assignmentInAssert",
@@ -124,7 +124,7 @@ void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& v
 }
 
 // checks if side effects happen on the variable prior to tmp
-void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *assertionScope)
+void CheckAssertImpl::checkVariableAssignment(const Token* assignTok, const Scope *assertionScope)
 {
     if (!assignTok->isAssignmentOp() && assignTok->tokType() != Token::eIncDecOp)
         return;
@@ -152,8 +152,21 @@ void CheckAssert::checkVariableAssignment(const Token* assignTok, const Scope *a
     // TODO: function calls on var
 }
 
-bool CheckAssert::inSameScope(const Token* returnTok, const Token* assignTok)
+bool CheckAssertImpl::inSameScope(const Token* returnTok, const Token* assignTok)
 {
     // TODO: even if a return is in the same scope, the assignment might not affect it.
     return returnTok->scope() == assignTok->scope();
 }
+
+void CheckAssert::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger)
+{
+    CheckAssertImpl checkAssert(&tokenizer, tokenizer.getSettings(), errorLogger);
+    checkAssert.assertWithSideEffects();
+}
+
+void CheckAssert::getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const
+{
+    CheckAssertImpl c(nullptr, settings, errorLogger);
+    c.sideEffectInAssertError(nullptr, "function");
+    c.assignmentInAssertError(nullptr, "var");
+}

lib/checkassert.cpp

@@ -23,6 +23,7 @@
 //---------------------------------------------------------------------------
 
 #include "check.h"
+#include "checkimpl.h"
 #include "config.h"
 #include "tokenize.h"
 
@@ -42,17 +43,23 @@ class Token;
 
 class CPPCHECKLIB CheckAssert : public Check {
 public:
-    CheckAssert() : Check(myName()) {}
+    CheckAssert() : Check("Assert") {}
 
 private:
-    CheckAssert(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
-        : Check(myName(), tokenizer, settings, errorLogger) {}
-
     /** run checks, the token list is not simplified */
-    void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override {
-        CheckAssert checkAssert(&tokenizer, tokenizer.getSettings(), errorLogger);
-        checkAssert.assertWithSideEffects();
+    void runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) override;
+
+    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
+
+    std::string classInfo() const override {
+        return "Warn if there are side effects in assert statements (since this cause different behaviour in debug/release builds).\n";
     }
+};
+
+class CheckAssertImpl: public CheckImpl {
+public:
+    CheckAssertImpl(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
+            : CheckImpl(tokenizer, settings, errorLogger) {}
 
     void assertWithSideEffects();
 
@@ -61,20 +68,6 @@ class CPPCHECKLIB CheckAssert : public Check {
 
     void sideEffectInAssertError(const Token *tok, const std::string& functionName);
     void assignmentInAssertError(const Token *tok, const std::string &varname);
-
-    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override {
-        CheckAssert c(nullptr, settings, errorLogger);
-        c.sideEffectInAssertError(nullptr, "function");
-        c.assignmentInAssertError(nullptr, "var");
-    }
-
-    static std::string myName() {
-        return "Assert";
-    }
-
-    std::string classInfo() const override {
-        return "Warn if there are side effects in assert statements (since this cause different behaviour in debug/release builds).\n";
-    }
 };
 /// @}
 //---------------------------------------------------------------------------