From 793ed6802939912f850d7f53e64fab9e17efa97d Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 10 May 2020 20:32:59 +0200 Subject: [PATCH] Refactorization: Moved code from header to source - from utils.h to new utils.cpp - from token.h to token.cpp - from valueflow.h to valueflow.cpp - from errorlogger.h to errorlogger.cpp --- lib/astutils.cpp | 1 + lib/cppcheck.vcxproj | 3 +- lib/cppcheck.vcxproj.filters | 8 ++- lib/errorlogger.cpp | 45 ++++++++++++++ lib/errorlogger.h | 44 +------------- lib/forwardanalyzer.cpp | 1 + lib/pathanalysis.cpp | 1 + lib/token.cpp | 78 ++++++++++++++++++++++++ lib/token.h | 72 +++------------------- lib/utils.cpp | 113 +++++++++++++++++++++++++++++++++++ lib/utils.h | 92 +--------------------------- lib/valueflow.cpp | 16 ++++- lib/valueflow.h | 12 +--- test/testvalueflow.cpp | 1 + 14 files changed, 277 insertions(+), 210 deletions(-) create mode 100644 lib/utils.cpp diff --git a/lib/astutils.cpp b/lib/astutils.cpp index ea1b88765..1e6a3c972 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -27,6 +27,7 @@ #include "token.h" #include "valueflow.h" +#include #include #include #include diff --git a/lib/cppcheck.vcxproj b/lib/cppcheck.vcxproj index 9a7a24198..7b830bf25 100644 --- a/lib/cppcheck.vcxproj +++ b/lib/cppcheck.vcxproj @@ -96,6 +96,7 @@ + @@ -558,4 +559,4 @@ xcopy "$(SolutionDir)platforms" "$(OutDir)platforms" /E /I /D /Y - + \ No newline at end of file diff --git a/lib/cppcheck.vcxproj.filters b/lib/cppcheck.vcxproj.filters index 7f09fadf8..88d82908c 100644 --- a/lib/cppcheck.vcxproj.filters +++ b/lib/cppcheck.vcxproj.filters @@ -170,6 +170,12 @@ Source Files + + Source Files + + + Source Files + @@ -338,4 +344,4 @@ - + \ No newline at end of file diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 9edcb1ee0..70b9c56e6 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -58,6 +58,51 @@ InternalError::InternalError(const Token *tok, const std::string &errorMsg, Type } } +std::string Severity::toString(Severity::SeverityType severity) +{ + switch (severity) { + case none: + return ""; + case error: + return "error"; + case warning: + return "warning"; + case style: + return "style"; + case performance: + return "performance"; + case portability: + return "portability"; + case information: + return "information"; + case debug: + return "debug"; + } + throw InternalError(nullptr, "Unknown severity"); +} +Severity::SeverityType Severity::fromString(const std::string& severity) +{ + if (severity.empty()) + return none; + if (severity == "none") + return none; + if (severity == "error") + return error; + if (severity == "warning") + return warning; + if (severity == "style") + return style; + if (severity == "performance") + return performance; + if (severity == "portability") + return portability; + if (severity == "information") + return information; + if (severity == "debug") + return debug; + return none; +} + ErrorLogger::ErrorMessage::ErrorMessage() : incomplete(false), severity(Severity::none), cwe(0U), inconclusive(false) { diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 18275cc32..9b1c47bf7 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -126,48 +126,8 @@ public: debug }; - static std::string toString(SeverityType severity) { - switch (severity) { - case none: - return ""; - case error: - return "error"; - case warning: - return "warning"; - case style: - return "style"; - case performance: - return "performance"; - case portability: - return "portability"; - case information: - return "information"; - case debug: - return "debug"; - } - throw InternalError(nullptr, "Unknown severity"); - } - static SeverityType fromString(const std::string &severity) { - if (severity.empty()) - return none; - if (severity == "none") - return none; - if (severity == "error") - return error; - if (severity == "warning") - return warning; - if (severity == "style") - return style; - if (severity == "performance") - return performance; - if (severity == "portability") - return portability; - if (severity == "information") - return information; - if (severity == "debug") - return debug; - return none; - } + static std::string toString(SeverityType severity); + static SeverityType fromString(const std::string& severity); }; diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 8903dab02..be843c9c1 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -5,6 +5,7 @@ #include "token.h" #include "valueptr.h" +#include #include struct ForwardTraversal { diff --git a/lib/pathanalysis.cpp b/lib/pathanalysis.cpp index c8ecc2a68..7cb7edd4b 100644 --- a/lib/pathanalysis.cpp +++ b/lib/pathanalysis.cpp @@ -3,6 +3,7 @@ #include "symboldatabase.h" #include "token.h" #include "valueflow.h" +#include const Scope* PathAnalysis::findOuterScope(const Scope * scope) { diff --git a/lib/token.cpp b/lib/token.cpp index 6e241a77b..8fbd36047 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -25,6 +25,7 @@ #include "symboldatabase.h" #include "utils.h" +#include #include #include #include @@ -2127,6 +2128,83 @@ std::shared_ptr Token::scopeInfo() const return mImpl->mScopeInfo; } +bool Token::hasKnownIntValue() const +{ + if (!mImpl->mValues) + return false; + return std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) { + return value.isKnown() && value.isIntValue(); + }); +} + +bool Token::hasKnownValue() const +{ + return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown)); +} + +bool Token::isImpossibleIntValue(const MathLib::bigint val) const +{ + if (!mImpl->mValues) + return false; + for (const auto& v : *mImpl->mValues) { + if (v.isIntValue() && v.isImpossible() && v.intvalue == val) + return true; + if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Lower && val > v.intvalue) + return true; + if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Upper && val < v.intvalue) + return true; + } + return false; +} + +const ValueFlow::Value* Token::getValue(const MathLib::bigint val) const +{ + if (!mImpl->mValues) + return nullptr; + const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) { + return value.isIntValue() && !value.isImpossible() && value.intvalue == val; + }); + return it == mImpl->mValues->end() ? nullptr : &*it; +} + +const ValueFlow::Value* Token::getMaxValue(bool condition) const +{ + if (!mImpl->mValues) + return nullptr; + const ValueFlow::Value* ret = nullptr; + for (const ValueFlow::Value& value : *mImpl->mValues) { + if (!value.isIntValue()) + continue; + if (value.isImpossible()) + continue; + if ((!ret || value.intvalue > ret->intvalue) && + ((value.condition != nullptr) == condition)) + ret = &value; + } + return ret; +} + +const ValueFlow::Value* Token::getMovedValue() const +{ + if (!mImpl->mValues) + return nullptr; + const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) { + return value.isMovedValue() && !value.isImpossible() && + value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable; + }); + return it == mImpl->mValues->end() ? nullptr : &*it; +} + +const ValueFlow::Value* Token::getContainerSizeValue(const MathLib::bigint val) const +{ + if (!mImpl->mValues) + return nullptr; + const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) { + return value.isContainerSizeValue() && !value.isImpossible() && value.intvalue == val; + }); + return it == mImpl->mValues->end() ? nullptr : &*it; +} + TokenImpl::~TokenImpl() { delete mOriginalName; diff --git a/lib/token.h b/lib/token.h index 588e4caf4..70d2df6d7 100644 --- a/lib/token.h +++ b/lib/token.h @@ -27,7 +27,6 @@ #include "templatesimplifier.h" #include "utils.h" -#include #include #include #include @@ -1016,84 +1015,27 @@ public: *mImpl->mOriginalName = name; } - bool hasKnownIntValue() const { - if (!mImpl->mValues) - return false; - return std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value &value) { - return value.isKnown() && value.isIntValue(); - }); - } - - bool hasKnownValue() const { - return mImpl->mValues && std::any_of(mImpl->mValues->begin(), mImpl->mValues->end(), std::mem_fn(&ValueFlow::Value::isKnown)); - } + bool hasKnownIntValue() const; + bool hasKnownValue() const; MathLib::bigint getKnownIntValue() const { return mImpl->mValues->front().intvalue; } - bool isImpossibleIntValue(const MathLib::bigint val) const { - if (!mImpl->mValues) - return false; - for (const auto &v: *mImpl->mValues) { - if (v.isIntValue() && v.isImpossible() && v.intvalue == val) - return true; - if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Lower && val > v.intvalue) - return true; - if (v.isIntValue() && v.bound == ValueFlow::Value::Bound::Upper && val < v.intvalue) - return true; - } - return false; - } + bool isImpossibleIntValue(const MathLib::bigint val) const; - const ValueFlow::Value * getValue(const MathLib::bigint val) const { - if (!mImpl->mValues) - return nullptr; - const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) { - return value.isIntValue() && !value.isImpossible() && value.intvalue == val; - }); - return it == mImpl->mValues->end() ? nullptr : &*it; - } + const ValueFlow::Value* getValue(const MathLib::bigint val) const; - const ValueFlow::Value * getMaxValue(bool condition) const { - if (!mImpl->mValues) - return nullptr; - const ValueFlow::Value *ret = nullptr; - for (const ValueFlow::Value &value : *mImpl->mValues) { - if (!value.isIntValue()) - continue; - if (value.isImpossible()) - continue; - if ((!ret || value.intvalue > ret->intvalue) && - ((value.condition != nullptr) == condition)) - ret = &value; - } - return ret; - } + const ValueFlow::Value* getMaxValue(bool condition) const; - const ValueFlow::Value * getMovedValue() const { - if (!mImpl->mValues) - return nullptr; - const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [](const ValueFlow::Value& value) { - return value.isMovedValue() && !value.isImpossible() && - value.moveKind != ValueFlow::Value::MoveKind::NonMovedVariable; - }); - return it == mImpl->mValues->end() ? nullptr : &*it; - } + const ValueFlow::Value* getMovedValue() const; const ValueFlow::Value * getValueLE(const MathLib::bigint val, const Settings *settings) const; const ValueFlow::Value * getValueGE(const MathLib::bigint val, const Settings *settings) const; const ValueFlow::Value * getInvalidValue(const Token *ftok, nonneg int argnr, const Settings *settings) const; - const ValueFlow::Value * getContainerSizeValue(const MathLib::bigint val) const { - if (!mImpl->mValues) - return nullptr; - const auto it = std::find_if(mImpl->mValues->begin(), mImpl->mValues->end(), [=](const ValueFlow::Value& value) { - return value.isContainerSizeValue() && !value.isImpossible() && value.intvalue == val; - }); - return it == mImpl->mValues->end() ? nullptr : &*it; - } + const ValueFlow::Value* getContainerSizeValue(const MathLib::bigint val) const; const Token *getValueTokenMaxStrLength() const; const Token *getValueTokenMinStrSize(const Settings *settings) const; diff --git a/lib/utils.cpp b/lib/utils.cpp new file mode 100644 index 000000000..1791f830d --- /dev/null +++ b/lib/utils.cpp @@ -0,0 +1,113 @@ +/* + * Cppcheck - A tool for static C/C++ code analysis + * Copyright (C) 2007-2020 Cppcheck team. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "utils.h" + +#include +#include + + +int caseInsensitiveStringCompare(const std::string &lhs, const std::string &rhs) +{ + if (lhs.size() != rhs.size()) + return (lhs.size() < rhs.size()) ? -1 : 1; + for (unsigned int i = 0; i < lhs.size(); ++i) { + const int c1 = std::toupper(lhs[i]); + const int c2 = std::toupper(rhs[i]); + if (c1 != c2) + return (c1 < c2) ? -1 : 1; + } + return 0; +} + +bool isValidGlobPattern(const std::string& pattern) +{ + for (std::string::const_iterator i = pattern.begin(); i != pattern.end(); ++i) { + if (*i == '*' || *i == '?') { + std::string::const_iterator j = i + 1; + if (j != pattern.end() && (*j == '*' || *j == '?')) { + return false; + } + } + } + return true; +} + +bool matchglob(const std::string& pattern, const std::string& name) +{ + const char* p = pattern.c_str(); + const char* n = name.c_str(); + std::stack > backtrack; + + for (;;) { + bool matching = true; + while (*p != '\0' && matching) { + switch (*p) { + case '*': + // Step forward until we match the next character after * + while (*n != '\0' && *n != p[1]) { + n++; + } + if (*n != '\0') { + // If this isn't the last possibility, save it for later + backtrack.push(std::make_pair(p, n)); + } + break; + case '?': + // Any character matches unless we're at the end of the name + if (*n != '\0') { + n++; + } else { + matching = false; + } + break; + default: + // Non-wildcard characters match literally + if (*n == *p) { + n++; + } else if (*n == '\\' && *p == '/') { + n++; + } else if (*n == '/' && *p == '\\') { + n++; + } else { + matching = false; + } + break; + } + p++; + } + + // If we haven't failed matching and we've reached the end of the name, then success + if (matching && *n == '\0') { + return true; + } + + // If there are no other paths to try, then fail + if (backtrack.empty()) { + return false; + } + + // Restore pointers from backtrack stack + p = backtrack.top().first; + n = backtrack.top().second; + backtrack.pop(); + + // Advance name pointer by one because the current position didn't work + n++; + } +} diff --git a/lib/utils.h b/lib/utils.h index 53478436b..b0a9d2116 100644 --- a/lib/utils.h +++ b/lib/utils.h @@ -21,11 +21,9 @@ #define utilsH //--------------------------------------------------------------------------- -#include #include #include #include -#include #include inline bool endsWith(const std::string &str, char c) @@ -98,95 +96,11 @@ inline static const char *getOrdinalText(int i) return "th"; } -inline static int caseInsensitiveStringCompare(const std::string &lhs, const std::string &rhs) -{ - if (lhs.size() != rhs.size()) - return (lhs.size() < rhs.size()) ? -1 : 1; - for (unsigned int i = 0; i < lhs.size(); ++i) { - const int c1 = std::toupper(lhs[i]); - const int c2 = std::toupper(rhs[i]); - if (c1 != c2) - return (c1 < c2) ? -1 : 1; - } - return 0; -} +CPPCHECKLIB int caseInsensitiveStringCompare(const std::string& lhs, const std::string& rhs); -inline static bool isValidGlobPattern(const std::string& pattern) -{ - for (std::string::const_iterator i = pattern.begin(); i != pattern.end(); ++i) { - if (*i == '*' || *i == '?') { - std::string::const_iterator j = i + 1; - if (j != pattern.end() && (*j == '*' || *j == '?')) { - return false; - } - } - } - return true; -} +CPPCHECKLIB bool isValidGlobPattern(const std::string& pattern); -inline static bool matchglob(const std::string& pattern, const std::string& name) -{ - const char* p = pattern.c_str(); - const char* n = name.c_str(); - std::stack > backtrack; - - for (;;) { - bool matching = true; - while (*p != '\0' && matching) { - switch (*p) { - case '*': - // Step forward until we match the next character after * - while (*n != '\0' && *n != p[1]) { - n++; - } - if (*n != '\0') { - // If this isn't the last possibility, save it for later - backtrack.push(std::make_pair(p, n)); - } - break; - case '?': - // Any character matches unless we're at the end of the name - if (*n != '\0') { - n++; - } else { - matching = false; - } - break; - default: - // Non-wildcard characters match literally - if (*n == *p) { - n++; - } else if (*n == '\\' && *p == '/') { - n++; - } else if (*n == '/' && *p == '\\') { - n++; - } else { - matching = false; - } - break; - } - p++; - } - - // If we haven't failed matching and we've reached the end of the name, then success - if (matching && *n == '\0') { - return true; - } - - // If there are no other paths to try, then fail - if (backtrack.empty()) { - return false; - } - - // Restore pointers from backtrack stack - p = backtrack.top().first; - n = backtrack.top().second; - backtrack.pop(); - - // Advance name pointer by one because the current position didn't work - n++; - } -} +CPPCHECKLIB bool matchglob(const std::string& pattern, const std::string& name); #define UNUSED(x) (void)(x) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 69794867c..445eeded0 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -6061,6 +6061,20 @@ std::string ValueFlow::Value::infoString() const throw InternalError(nullptr, "Invalid ValueFlow Value type"); } +const char* ValueFlow::Value::toString(MoveKind moveKind) +{ + switch (moveKind) { + case MoveKind::NonMovedVariable: + return "NonMovedVariable"; + case MoveKind::MovedVariable: + return "MovedVariable"; + case MoveKind::ForwardedVariable: + return "ForwardedVariable"; + } + return ""; +} + + const ValueFlow::Value *ValueFlow::valueFlowConstantFoldAST(Token *expr, const Settings *settings) { if (expr && expr->values().empty()) { @@ -6143,4 +6157,4 @@ std::string ValueFlow::eitherTheConditionIsRedundant(const Token *condition) return "Either the switch case '" + expr + "' is redundant"; } return "Either the condition '" + condition->expressionString() + "' is redundant"; -} +} \ No newline at end of file diff --git a/lib/valueflow.h b/lib/valueflow.h index 7113505ef..b20d26e3a 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -256,17 +256,7 @@ namespace ValueFlow { enum class LifetimeScope { Local, Argument } lifetimeScope; - static const char * toString(MoveKind moveKind) { - switch (moveKind) { - case MoveKind::NonMovedVariable: - return "NonMovedVariable"; - case MoveKind::MovedVariable: - return "MovedVariable"; - case MoveKind::ForwardedVariable: - return "ForwardedVariable"; - } - return ""; - } + static const char* toString(MoveKind moveKind); /** How known is this value */ enum class ValueKind { diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index b005a5ccb..5f491a1f0 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -25,6 +25,7 @@ #include "valueflow.h" #include +#include #include #include #include