From ac7647fcd8b2e10c2e277b6643e62640d7cca647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Sat, 9 Jan 2021 20:32:38 +0100 Subject: [PATCH] some self-check suppression cleanups (#3032) --- .github/workflows/CI-unixish.yml | 6 +++--- .travis_suppressions | 10 ---------- cli/cmdlineparser.cpp | 4 ++++ cli/cppcheckexecutor.cpp | 2 +- cli/threadexecutor.h | 2 +- lib/checkbufferoverrun.cpp | 8 ++++---- lib/checkbufferoverrun.h | 2 +- lib/checkclass.cpp | 1 + lib/checknullpointer.cpp | 2 +- lib/checkuninitvar.cpp | 2 +- lib/checkunusedvar.cpp | 6 ++---- lib/ctu.cpp | 2 +- lib/ctu.h | 4 ++-- lib/exprengine.cpp | 2 +- lib/tokenize.cpp | 5 +++-- lib/tokenize.h | 4 ++-- lib/tokenlist.cpp | 4 ++-- lib/tokenlist.h | 2 +- test/testother.cpp | 1 + test/testtoken.cpp | 8 ++++++++ 20 files changed, 40 insertions(+), 37 deletions(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index 4b77e87df..5e342ae0f 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -110,12 +110,12 @@ jobs: make -j$(nproc) -s CPPFLAGS="-DCHECK_INTERNAL" CXXFLAGS="-g -O2" MATCHCOMPILER=yes VERIFY=1 # self check lib/cli mkdir b1 - ./cppcheck -q -j$(nproc) --template=gcc --cppcheck-build-dir=b1 -D__CPPCHECK__ --error-exitcode=1 --inline-suppr --suppressions-list=.travis_suppressions --library=cppcheck-lib --addon=naming.json -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ -Icli --inconclusive --enable=style,performance,portability,warning,internal --exception-handling cli lib + ./cppcheck -q -j$(nproc) --template=selfcheck --cppcheck-build-dir=b1 -D__CPPCHECK__ --error-exitcode=1 --inline-suppr --suppressions-list=.travis_suppressions --library=cppcheck-lib --addon=naming.json -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ -Icli --inconclusive --enable=style,performance,portability,warning,internal --exception-handling cli lib # check gui with qt settings mkdir b2 - ./cppcheck -q -j$(nproc) --template=gcc --cppcheck-build-dir=b2 -D__CPPCHECK__ -DQT_VERSION=0x050000 --error-exitcode=1 --inline-suppr --suppressions-list=.travis_suppressions --library=qt --addon=naming.json -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --enable=style,performance,portability,warning,internal --exception-handling gui/*.cpp + ./cppcheck -q -j$(nproc) --template=selfcheck --cppcheck-build-dir=b2 -D__CPPCHECK__ -DQT_VERSION=0x050000 --error-exitcode=1 --inline-suppr --suppressions-list=.travis_suppressions --library=qt --addon=naming.json -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ --enable=style,performance,portability,warning,internal --exception-handling gui/*.cpp # self check test and tools - ./cppcheck -q -j$(nproc) --template=gcc -D__CPPCHECK__ --error-exitcode=1 --inline-suppr --suppressions-list=.travis_suppressions --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ -Icli -Igui --inconclusive --enable=style,performance,portability,warning,internal --exception-handling test/*.cpp tools + ./cppcheck -q -j$(nproc) --template=selfcheck -D__CPPCHECK__ --error-exitcode=1 --inline-suppr --suppressions-list=.travis_suppressions --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2/ -Icli -Igui --inconclusive --enable=style,performance,portability,warning,internal --exception-handling test/*.cpp tools - name: Build triage on ubuntu if: matrix.os == 'ubuntu-20.04' diff --git a/.travis_suppressions b/.travis_suppressions index f802c6392..5773b58e8 100644 --- a/.travis_suppressions +++ b/.travis_suppressions @@ -1,22 +1,12 @@ unusedPrivateFunction:test/testbufferoverrun.cpp unusedPrivateFunction:test/testcmdlineparser.cpp -redundantNextPrevious:test/testtoken.cpp -simplePatternError:test/testtoken.cpp -noValidConfiguration shadowFunction functionConst functionStatic bitwiseOnBoolean -hidingInheritedPublic # temporary suppressions - fix the warnings! -duplicateBranch:lib/checkunusedvar.cpp -duplicateBranch:lib/tokenize.cpp missingOverride -redundantAssignment:lib/tokenlist.cpp -unreachableCode:lib/checkbufferoverrun.cpp -unreachableCode:lib/checkclass.cpp -unreachableCode:test/testother.cpp unusedPrivateFunction:lib/checkbufferoverrun.h unusedPrivateFunction:test/test*.cpp useStlAlgorithm diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index b9c40ad84..41b405383 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -849,6 +849,10 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) mSettings->templateFormat = "{file} +{line}: {severity}: {message}"; else if (mSettings->templateFormat == "cppcheck1") mSettings->templateFormat = "{callstack}: ({severity}{inconclusive:, inconclusive}) {message}"; + else if (mSettings->templateFormat == "selfcheck") { + mSettings->templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]\\n{code}"; + mSettings->templateLocation = "{file}:{line}:{column}: note: {info}\\n{code}"; + } } else if (std::strcmp(argv[i], "--template-location") == 0 || diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 8dc433b80..a0dc0a8df 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -1169,7 +1169,7 @@ bool CppCheckExecutor::tryLoadLibrary(Library& destination, const char* basepath /** * Execute a shell command and read the output from it. Returns true if command terminated successfully. */ -// cppcheck-suppress passedByValue +// cppcheck-suppress passedByValue - "exe" copy needed in _WIN32 code bool CppCheckExecutor::executeCommand(std::string exe, std::vector args, const std::string &redirect, std::string *output) { output->clear(); diff --git a/cli/threadexecutor.h b/cli/threadexecutor.h index e6b244ad8..d461d9677 100644 --- a/cli/threadexecutor.h +++ b/cli/threadexecutor.h @@ -27,7 +27,7 @@ #include #include -#if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) && !defined(__CYGWIN__) +#if ((defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) && !defined(__CYGWIN__)) || defined(__CPPCHECK__) #define THREADING_MODEL_FORK #elif defined(_WIN32) #define THREADING_MODEL_WIN diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 3a328c19c..20132637b 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -826,19 +826,19 @@ bool CheckBufferOverrun::analyseWholeProgram(const CTU::FileInfo *ctu, const std if (!fi) continue; for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafeArrayIndex) - foundErrors |= analyseWholeProgram1(ctu, callsMap, unsafeUsage, 1, errorLogger); + foundErrors |= analyseWholeProgram1(callsMap, unsafeUsage, 1, errorLogger); for (const CTU::FileInfo::UnsafeUsage &unsafeUsage : fi->unsafePointerArith) - foundErrors |= analyseWholeProgram1(ctu, callsMap, unsafeUsage, 2, errorLogger); + foundErrors |= analyseWholeProgram1(callsMap, unsafeUsage, 2, errorLogger); } return foundErrors; } -bool CheckBufferOverrun::analyseWholeProgram1(const CTU::FileInfo *ctu, const std::map> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger) +bool CheckBufferOverrun::analyseWholeProgram1(const std::map> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger) { const CTU::FileInfo::FunctionCall *functionCall = nullptr; const std::list &locationList = - ctu->getErrorPath(CTU::FileInfo::InvalidValueType::bufferOverflow, + CTU::FileInfo::getErrorPath(CTU::FileInfo::InvalidValueType::bufferOverflow, unsafeUsage, callsMap, "Using argument ARG", diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 662225673..00bd52ec6 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -137,7 +137,7 @@ private: static bool isCtuUnsafePointerArith(const Check *check, const Token *argtok, MathLib::bigint *offset); Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const OVERRIDE; - bool analyseWholeProgram1(const CTU::FileInfo *ctu, const std::map> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger); + static bool analyseWholeProgram1(const std::map> &callsMap, const CTU::FileInfo::UnsafeUsage &unsafeUsage, int type, ErrorLogger &errorLogger); static std::string myName() { diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 6172b122b..d38c1661d 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -2497,6 +2497,7 @@ void CheckClass::checkCopyCtorAndEqOperator() // The message must be clarified. How is the behaviour different? return; + // cppcheck-suppress unreachableCode - remove when code is enabled again if (!mSettings->isEnabled(Settings::WARNING)) return; diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 9da2180f9..a66ec5766 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -591,7 +591,7 @@ bool CheckNullPointer::analyseWholeProgram(const CTU::FileInfo *ctu, const std:: break; const std::list &locationList = - ctu->getErrorPath(CTU::FileInfo::InvalidValueType::null, + CTU::FileInfo::getErrorPath(CTU::FileInfo::InvalidValueType::null, unsafeUsage, callsMap, "Dereferencing argument ARG that is null", diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index c1b9d6454..49681c69e 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1446,7 +1446,7 @@ bool CheckUninitVar::analyseWholeProgram(const CTU::FileInfo *ctu, const std::li const CTU::FileInfo::FunctionCall *functionCall = nullptr; const std::list &locationList = - ctu->getErrorPath(CTU::FileInfo::InvalidValueType::uninit, + CTU::FileInfo::getErrorPath(CTU::FileInfo::InvalidValueType::uninit, unsafeUsage, callsMap, "Using argument ARG", diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 441008b74..02e5c0a21 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -521,15 +521,13 @@ static const Token* doAssignment(Variables &variables, const Token *tok, bool de if (var1->_assignments.find(scope) == var1->_assignments.end() || scope->type == Scope::eSwitch) { // nothing to replace + // cppcheck-suppress duplicateBranch - remove when TODO below is address if (var1->_assignments.empty()) replace = false; // this variable has previous assignments else { - /** - * @todo determine if existing aliases should be replaced or merged - */ - + // TODO: determine if existing aliases should be replaced or merged replace = false; } } diff --git a/lib/ctu.cpp b/lib/ctu.cpp index a152112b5..bff64db5b 100644 --- a/lib/ctu.cpp +++ b/lib/ctu.cpp @@ -532,7 +532,7 @@ std::list CTU::FileInfo::getErrorPath(InvalidValueTy const std::map> &callsMap, const char info[], const FunctionCall * * const functionCallPtr, - bool warning) const + bool warning) { std::list locationList; diff --git a/lib/ctu.h b/lib/ctu.h index 0727fbe3f..44cdc76aa 100644 --- a/lib/ctu.h +++ b/lib/ctu.h @@ -116,12 +116,12 @@ namespace CTU { void loadFromXml(const tinyxml2::XMLElement *xmlElement); std::map> getCallsMap() const; - std::list getErrorPath(InvalidValueType invalidValue, + static std::list getErrorPath(InvalidValueType invalidValue, const UnsafeUsage &unsafeUsage, const std::map> &callsMap, const char info[], const FunctionCall * * const functionCallPtr, - bool warning) const; + bool warning); }; extern int maxCtuDepth; diff --git a/lib/exprengine.cpp b/lib/exprengine.cpp index e7d033553..bf8d24754 100644 --- a/lib/exprengine.cpp +++ b/lib/exprengine.cpp @@ -750,7 +750,7 @@ namespace { std::vector constraints; }; - void parsestr(const std::string &s, std::vector *importData) const { + static void parsestr(const std::string &s, std::vector *importData) { std::string line; std::istringstream istr(s); while (std::getline(istr, line)) { diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index b4e9d8298..721cc4243 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -3182,7 +3182,7 @@ static bool setVarIdParseDeclaration(const Token **tok, const std::map >& structMembers, - nonneg int *varId) + nonneg int *varId) const { Token *tok = *tok1; @@ -9572,7 +9572,7 @@ static bool isCPPAttribute(const Token * tok) return Token::simpleMatch(tok, "[ [") && tok->link() && tok->link()->previous() == tok->linkAt(1); } -void Tokenizer::reportUnknownMacros() +void Tokenizer::reportUnknownMacros() const { // Report unknown macros used in expressions "%name% %num%" for (const Token *tok = tokens(); tok; tok = tok->next()) { @@ -12005,6 +12005,7 @@ void Tokenizer::simplifyNamespaceAliases() continue; } else { // conflicting declaration (syntax error) + // cppcheck-suppress duplicateBranch - remove when TODO below is addressed if (endScope == scope) { // delete conflicting declaration tok2 = deleteAlias(tok2->previous()); diff --git a/lib/tokenize.h b/lib/tokenize.h index 5a21be2aa..746323531 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -664,7 +664,7 @@ private: void validate() const; /** Detect unknown macros and throw unknownMacro */ - void reportUnknownMacros(); + void reportUnknownMacros() const; /** Detect garbage code and call syntaxError() if found. */ void findGarbageCode() const; @@ -809,7 +809,7 @@ private: void setVarIdStructMembers(Token **tok1, std::map >& structMembers, - nonneg int *varId); + nonneg int *varId) const; void setVarIdClassFunction(const std::string &classname, Token * const startToken, diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp index 8fee2c2f0..1ed0dd809 100644 --- a/lib/tokenlist.cpp +++ b/lib/tokenlist.cpp @@ -1465,7 +1465,7 @@ static Token * createAstAtToken(Token *tok, bool cpp) compileExpression(tok2, state1); if (Token::Match(tok2, ";|)")) break; - init1 = nullptr; // cppcheck-suppress redundantAssignment ; FALSE POSITIVE + init1 = nullptr; } if (!tok2) // #7109 invalid code return nullptr; @@ -1614,7 +1614,7 @@ static Token * createAstAtToken(Token *tok, bool cpp) return tok; } -void TokenList::createAst() +void TokenList::createAst() const { for (Token *tok = mTokensFrontBack.front; tok; tok = tok ? tok->next() : nullptr) { tok = createAstAtToken(tok, isCPP()); diff --git a/lib/tokenlist.h b/lib/tokenlist.h index 44912298a..2661e9b20 100644 --- a/lib/tokenlist.h +++ b/lib/tokenlist.h @@ -156,7 +156,7 @@ public: /** * Create abstract syntax tree. */ - void createAst(); + void createAst() const; /** * Check abstract syntax tree. diff --git a/test/testother.cpp b/test/testother.cpp index d66fda43e..a8a06b5cb 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -7518,6 +7518,7 @@ private: return; // FIXME: temporary hack // Simple tests + // cppcheck-suppress unreachableCode - remove when code is enabled again check("void f() {\n" " char a[10];\n" " memcpy(a, foo, bar);\n" diff --git a/test/testtoken.cpp b/test/testtoken.cpp index 26429bcef..44ef459ca 100644 --- a/test/testtoken.cpp +++ b/test/testtoken.cpp @@ -120,11 +120,13 @@ private: Token *last = token->tokAt(2); ASSERT_EQUALS(token->str(), "1"); ASSERT_EQUALS(token->next()->str(), "2"); + // cppcheck-suppress redundantNextPrevious - this is itentional ASSERT_EQUALS(token->tokAt(2)->str(), "3"); ASSERT_EQUALS_MSG(true, last->next() == nullptr, "Null was expected"); ASSERT_EQUALS(last->str(), "3"); ASSERT_EQUALS(last->previous()->str(), "2"); + // cppcheck-suppress redundantNextPrevious - this is itentional ASSERT_EQUALS(last->tokAt(-2)->str(), "1"); ASSERT_EQUALS_MSG(true, token->previous() == nullptr, "Null was expected"); @@ -719,18 +721,24 @@ private: void matchOr() const { givenACodeSampleToTokenize bitwiseOr(";|;", true); + // cppcheck-suppress simplePatternError - this is itentional ASSERT_EQUALS(true, Token::Match(bitwiseOr.tokens(), "; %or%")); ASSERT_EQUALS(true, Token::Match(bitwiseOr.tokens(), "; %op%")); + // cppcheck-suppress simplePatternError - this is itentional ASSERT_EQUALS(false, Token::Match(bitwiseOr.tokens(), "; %oror%")); givenACodeSampleToTokenize bitwiseOrAssignment(";|=;"); + // cppcheck-suppress simplePatternError - this is itentional ASSERT_EQUALS(false, Token::Match(bitwiseOrAssignment.tokens(), "; %or%")); ASSERT_EQUALS(true, Token::Match(bitwiseOrAssignment.tokens(), "; %op%")); + // cppcheck-suppress simplePatternError - this is itentional ASSERT_EQUALS(false, Token::Match(bitwiseOrAssignment.tokens(), "; %oror%")); givenACodeSampleToTokenize logicalOr(";||;", true); + // cppcheck-suppress simplePatternError - this is itentional ASSERT_EQUALS(false, Token::Match(logicalOr.tokens(), "; %or%")); ASSERT_EQUALS(true, Token::Match(logicalOr.tokens(), "; %op%")); + // cppcheck-suppress simplePatternError - this is itentional ASSERT_EQUALS(true, Token::Match(logicalOr.tokens(), "; %oror%")); ASSERT_EQUALS(true, Token::Match(logicalOr.tokens(), "; &&|%oror%")); ASSERT_EQUALS(true, Token::Match(logicalOr.tokens(), "; %oror%|&&"));