CWE: refactoring. use constants instead of magic numbers.

Author: danmar

lib/check.h

@@ -33,6 +33,11 @@
 /// @addtogroup Core
 /// @{
 
+struct CWE {
+    explicit CWE(unsigned short ID) : id(ID) {}
+    unsigned short id;
+};
+
 /**
  * @brief Interface class that cppcheck uses to communicate with the checks.
  * All checking classes must inherit from this class
@@ -111,27 +116,27 @@ class CPPCHECKLIB Check {
     /** report an error */
     template<typename T, typename U>
     void reportError(const Token *tok, const Severity::SeverityType severity, const T id, const U msg) {
-        reportError(tok, severity, id, msg, 0U, false);
+        reportError(tok, severity, id, msg, CWE(0U), false);
     }
 
     /** report an error */
     template<typename T, typename U>
-    void reportError(const Token *tok, const Severity::SeverityType severity, const T id, const U msg, unsigned int cwe, bool inconclusive) {
+    void reportError(const Token *tok, const Severity::SeverityType severity, const T id, const U msg, const CWE &cwe, bool inconclusive) {
         std::list<const Token *> callstack(1, tok);
         reportError(callstack, severity, id, msg, cwe, inconclusive);
     }
 
     /** report an error */
     template<typename T, typename U>
     void reportError(const std::list<const Token *> &callstack, Severity::SeverityType severity, const T id, const U msg) {
-        reportError(callstack, severity, id, msg, 0U, false);
+        reportError(callstack, severity, id, msg, CWE(0U), false);
     }
 
     /** report an error */
     template<typename T, typename U>
-    void reportError(const std::list<const Token *> &callstack, Severity::SeverityType severity, const T id, const U msg, unsigned int cwe, bool inconclusive) {
+    void reportError(const std::list<const Token *> &callstack, Severity::SeverityType severity, const T id, const U msg, const CWE &cwe, bool inconclusive) {
         ErrorLogger::ErrorMessage errmsg(callstack, _tokenizer?&_tokenizer->list:0, severity, id, msg, inconclusive);
-        errmsg._cwe = cwe;
+        errmsg._cwe = cwe.id;
         if (_errorLogger)
             _errorLogger->reportErr(errmsg);
         else

lib/checkautovariables.cpp

@@ -33,6 +33,8 @@ namespace {
     static CheckAutoVariables instance;
 }
 
+static const CWE CWE562(562U);
+static const CWE CWE590(590U);
 
 bool CheckAutoVariables::isPtrArg(const Token *tok)
 {
@@ -287,12 +289,12 @@ void CheckAutoVariables::returnPointerToLocalArray()
 
 void CheckAutoVariables::errorReturnAddressToAutoVariable(const Token *tok)
 {
-    reportError(tok, Severity::error, "returnAddressOfAutoVariable", "Address of an auto-variable returned.", 562U, false);
+    reportError(tok, Severity::error, "returnAddressOfAutoVariable", "Address of an auto-variable returned.", CWE562, false);
 }
 
 void CheckAutoVariables::errorReturnPointerToLocalArray(const Token *tok)
 {
-    reportError(tok, Severity::error, "returnLocalVariable", "Pointer to local array variable returned.", 562, false);
+    reportError(tok, Severity::error, "returnLocalVariable", "Pointer to local array variable returned.", CWE562, false);
 }
 
 void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inconclusive)
@@ -303,15 +305,15 @@ void CheckAutoVariables::errorAutoVariableAssignment(const Token *tok, bool inco
                     "Dangerous assignment - the function parameter is assigned the address of a local "
                     "auto-variable. Local auto-variables are reserved from the stack which "
                     "is freed when the function ends. So the pointer to a local variable "
-                    "is invalid after the function ends.", 562U, false);
+                    "is invalid after the function ends.", CWE562, false);
     } else {
         reportError(tok, Severity::error, "autoVariables",
                     "Address of local auto-variable assigned to a function parameter.\n"
                     "Function parameter is assigned the address of a local auto-variable. "
                     "Local auto-variables are reserved from the stack which is freed when "
                     "the function ends. The address is invalid after the function ends and it "
                     "might 'leak' from the function through the parameter.",
-                    562U,
+                    CWE562,
                     true);
     }
 }
@@ -322,7 +324,7 @@ void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok,
                 "Address of function parameter '" + varname + "' returned.\n"
                 "Address of the function parameter '" + varname + "' becomes invalid after the function exits because "
                 "function parameters are stored on the stack which is freed when the function exits. Thus the returned "
-                "value is invalid.", 562U, false);
+                "value is invalid.", CWE562, false);
 }
 
 void CheckAutoVariables::errorUselessAssignmentArg(const Token *tok)
@@ -492,12 +494,12 @@ void CheckAutoVariables::returnReference()
 
 void CheckAutoVariables::errorReturnReference(const Token *tok)
 {
-    reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", 562U, false);
+    reportError(tok, Severity::error, "returnReference", "Reference to auto variable returned.", CWE562, false);
 }
 
 void CheckAutoVariables::errorReturnTempReference(const Token *tok)
 {
-    reportError(tok, Severity::error, "returnTempReference", "Reference to temporary returned.", 562U, false);
+    reportError(tok, Severity::error, "returnTempReference", "Reference to temporary returned.", CWE562, false);
 }
 
 void CheckAutoVariables::errorInvalidDeallocation(const Token *tok)
@@ -507,5 +509,5 @@ void CheckAutoVariables::errorInvalidDeallocation(const Token *tok)
                 "autovarInvalidDeallocation",
                 "Deallocation of an auto-variable results in undefined behaviour.\n"
                 "The deallocation of an auto-variable results in undefined behaviour. You should only free memory "
-                "that has been allocated dynamically.", 590U, false);
+                "that has been allocated dynamically.", CWE590, false);
 }

lib/checkbool.cpp

@@ -28,6 +28,8 @@ namespace {
     CheckBool instance;
 }
 
+static const CWE CWE571(571);
+static const CWE CWE587(587);
 
 static bool astIsBool(const Token *expr)
 {
@@ -106,7 +108,7 @@ void CheckBool::bitwiseOnBooleanError(const Token *tok, const std::string &varna
 {
     reportError(tok, Severity::style, "bitwiseOnBoolean",
                 "Boolean variable '" + varname + "' is used in bitwise operation. Did you mean '" + op + "'?",
-                0U,
+                CWE(0),
                 true);
 }
 
@@ -352,7 +354,7 @@ void CheckBool::checkAssignBoolToPointer()
 void CheckBool::assignBoolToPointerError(const Token *tok)
 {
     reportError(tok, Severity::error, "assignBoolToPointer",
-                "Boolean value assigned to pointer.", 587U, false);
+                "Boolean value assigned to pointer.", CWE587, false);
 }
 
 //-----------------------------------------------------------------------------
@@ -473,7 +475,7 @@ void CheckBool::pointerArithBoolError(const Token *tok)
                 Severity::error,
                 "pointerArithBool",
                 "Converting pointer arithmetic result to bool. The bool is always true unless there is undefined behaviour.\n"
-                "Converting pointer arithmetic result to bool. The boolean result is always true unless there is pointer arithmetic overflow, and overflow is undefined behaviour. Probably a dereference is forgotten.", 571U, false);
+                "Converting pointer arithmetic result to bool. The boolean result is always true unless there is pointer arithmetic overflow, and overflow is undefined behaviour. Probably a dereference is forgotten.", CWE571, false);
 }
 
 void CheckBool::checkAssignBoolToFloat()

lib/checkboost.cpp

@@ -24,6 +24,8 @@ namespace {
     CheckBoost instance;
 }
 
+static const CWE CWE664(664);
+
 void CheckBoost::checkBoostForeachModification()
 {
     const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
@@ -55,6 +57,6 @@ void CheckBoost::checkBoostForeachModification()
 void CheckBoost::boostForeachError(const Token *tok)
 {
     reportError(tok, Severity::error, "boostForeachError",
-                "BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container inside.", 664U, false
+                "BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container inside.", CWE664, false
                );
 }

lib/checkbufferoverrun.cpp

@@ -43,6 +43,13 @@ namespace {
 
 //---------------------------------------------------------------------------
 
+// CWE ids used:
+static const CWE CWE119(119U);
+static const CWE CWE131(131U);
+static const CWE CWE788(788U);
+
+//---------------------------------------------------------------------------
+
 static void makeArrayIndexOutOfBoundsError(std::ostream& oss, const CheckBufferOverrun::ArrayInfo &arrayInfo, const std::vector<MathLib::bigint> &index)
 {
     oss << "Array '" << arrayInfo.varname();
@@ -61,7 +68,7 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra
 {
     std::ostringstream oss;
     makeArrayIndexOutOfBoundsError(oss, arrayInfo, index);
-    reportError(tok, Severity::error, "arrayIndexOutOfBounds", oss.str(), 788U, false);
+    reportError(tok, Severity::error, "arrayIndexOutOfBounds", oss.str(), CWE788, false);
 }
 
 void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const ArrayInfo &arrayInfo, const std::vector<ValueFlow::Value> &index)
@@ -92,7 +99,7 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra
         std::list<const Token *> callstack;
         callstack.push_back(tok);
         callstack.push_back(condition);
-        reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), 0U, false);
+        reportError(callstack, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), CWE(0U), false);
     } else {
         std::ostringstream errmsg;
         errmsg << "Array '" << arrayInfo.varname();
@@ -133,7 +140,7 @@ static std::string bufferOverrunMessage(std::string varnames)
 
 void CheckBufferOverrun::bufferOverrunError(const Token *tok, const std::string &varnames)
 {
-    reportError(tok, Severity::error, "bufferAccessOutOfBounds", bufferOverrunMessage(varnames), 788U, false);
+    reportError(tok, Severity::error, "bufferAccessOutOfBounds", bufferOverrunMessage(varnames), CWE788, false);
 }
 
 
@@ -177,7 +184,7 @@ void CheckBufferOverrun::outOfBoundsError(const Token *tok, const std::string &w
     if (show_size_info)
         oss << ": Supplied size " << supplied_size << " is larger than actual size " << actual_size;
     oss << '.';
-    reportError(tok, Severity::error, "outOfBounds", oss.str(), 788U, false);
+    reportError(tok, Severity::error, "outOfBounds", oss.str(), CWE788, false);
 }
 
 void CheckBufferOverrun::pointerOutOfBoundsError(const Token *tok, const Token *index, const MathLib::bigint indexvalue)
@@ -221,12 +228,12 @@ void CheckBufferOverrun::terminateStrncpyError(const Token *tok, const std::stri
                 "The buffer '" + varname + "' may not be null-terminated after the call to strncpy().\n"
                 "If the source string's size fits or exceeds the given size, strncpy() does not add a "
                 "zero at the end of the buffer. This causes bugs later in the code if the code "
-                "assumes buffer is null-terminated.", 0U, true);
+                "assumes buffer is null-terminated.", CWE(0U), true);
 }
 
 void CheckBufferOverrun::cmdLineArgsError(const Token *tok)
 {
-    reportError(tok, Severity::error, "insecureCmdLineArgs", "Buffer overrun possible for long command line arguments.", 119U, false);
+    reportError(tok, Severity::error, "insecureCmdLineArgs", "Buffer overrun possible for long command line arguments.", CWE119, false);
 }
 
 void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const std::string &varname, const std::string &function)
@@ -235,7 +242,7 @@ void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const st
                                "The buffer '" + varname + "' is not null-terminated after the call to " + function + "(). "
                                "This will cause bugs later in the code if the code assumes the buffer is null-terminated.";
 
-    reportError(tok, Severity::warning, "bufferNotZeroTerminated", errmsg, 0U, true);
+    reportError(tok, Severity::warning, "bufferNotZeroTerminated", errmsg, CWE(0U), true);
 }
 
 void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname)
@@ -248,7 +255,7 @@ void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token *tok)
     reportError(tok, Severity::error, "negativeMemoryAllocationSize",
                 "Memory allocation size is negative.\n"
                 "Memory allocation size is negative."
-                "Negative allocation size has no specified behaviour.", 131U, false);
+                "Negative allocation size has no specified behaviour.", CWE131, false);
 }
 
 //---------------------------------------------------------------------------
@@ -1717,7 +1724,7 @@ void CheckBufferOverrun::negativeIndexError(const Token *tok, const ValueFlow::V
     ostr << "Array index " << index.intvalue << " is out of bounds.";
     if (index.condition)
         ostr << " Otherwise there is useless condition at line " << index.condition->linenr() << ".";
-    reportError(tok, index.condition ? Severity::warning : Severity::error, "negativeIndex", ostr.str(), 0U, index.inconclusive);
+    reportError(tok, index.condition ? Severity::warning : Severity::error, "negativeIndex", ostr.str(), CWE(0U), index.inconclusive);
 }
 
 CheckBufferOverrun::ArrayInfo::ArrayInfo()