From 97ffec85c0faa20e9caf2d207f08620b6c703e6d Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Fri, 12 Jan 2018 10:24:01 +0300 Subject: [PATCH] Fixed #7502 (Correct exit code if never used function is found) (#1026) --- cli/cppcheckexecutor.cpp | 3 ++- lib/check.h | 4 +++- lib/checkbufferoverrun.cpp | 5 ++++- lib/checkbufferoverrun.h | 2 +- lib/checkunusedfunctions.cpp | 10 +++++++--- lib/checkunusedfunctions.h | 5 +++-- lib/cppcheck.cpp | 6 ++++-- lib/cppcheck.h | 5 +++-- test/testsuppressions.cpp | 3 ++- test/testunusedfunctions.cpp | 6 +++++- 10 files changed, 34 insertions(+), 15 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 54bb8c4ad..ed81321e7 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -888,7 +888,8 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck, int /*argc*/, const cha c++; } } - cppcheck.analyseWholeProgram(); + if (cppcheck.analyseWholeProgram()) + returnValue++; } else if (!ThreadExecutor::isEnabled()) { std::cout << "No thread support yet implemented for this platform." << std::endl; } else { diff --git a/lib/check.h b/lib/check.h index 193ccf323..e9c4eecaf 100644 --- a/lib/check.h +++ b/lib/check.h @@ -109,10 +109,12 @@ public: return nullptr; } - virtual void analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) { + // Return true if an error is reported. + virtual bool analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) { (void)fileInfo; (void)settings; (void)errorLogger; + return false; } protected: diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 1e3cf7a9e..10949a6d3 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -2022,8 +2022,9 @@ Check::FileInfo * CheckBufferOverrun::loadFileInfoFromXml(const tinyxml2::XMLEle } -void CheckBufferOverrun::analyseWholeProgram(const std::list &fileInfo, const Settings&, ErrorLogger &errorLogger) +bool CheckBufferOverrun::analyseWholeProgram(const std::list &fileInfo, const Settings&, ErrorLogger &errorLogger) { + bool errors = false; // Merge all fileInfo MyFileInfo all; for (std::list::const_iterator it = fileInfo.begin(); it != fileInfo.end(); ++it) { @@ -2069,8 +2070,10 @@ void CheckBufferOverrun::analyseWholeProgram(const std::list & "arrayIndexOutOfBounds", CWE788, false); errorLogger.reportErr(errmsg); + errors = true; } } + return errors; } unsigned int CheckBufferOverrun::sizeOfType(const Token *type) const diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 1cf6f1970..d5a528a31 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -244,7 +244,7 @@ public: Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const; /** @brief Analyse all file infos for all TU */ - void analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger); + bool analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger); /** * Calculates sizeof value for given type. diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index 949b4c34b..614ebb2d2 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -237,8 +237,9 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const char Fi -void CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) +bool CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings& settings) { + bool errors = false; for (std::map::const_iterator it = _functions.begin(); it != _functions.end(); ++it) { const FunctionUsage &func = it->second; if (func.usedOtherFile || func.filename.empty()) @@ -252,15 +253,18 @@ void CheckUnusedFunctions::check(ErrorLogger * const errorLogger, const Settings if (func.filename != "+") filename = func.filename; unusedFunctionError(errorLogger, filename, func.lineNumber, it->first); + errors = true; } else if (! func.usedOtherFile) { /** @todo add error message "function is only used in it can be static" */ /* std::ostringstream errmsg; errmsg << "The function '" << it->first << "' is only used in the file it was declared in so it should have local linkage."; _errorLogger->reportErr( errmsg.str() ); + errors = true; */ } } + return errors; } void CheckUnusedFunctions::unusedFunctionError(ErrorLogger * const errorLogger, @@ -291,10 +295,10 @@ Check::FileInfo *CheckUnusedFunctions::getFileInfo(const Tokenizer *tokenizer, c return nullptr; } -void CheckUnusedFunctions::analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) +bool CheckUnusedFunctions::analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger) { (void)fileInfo; - check(&errorLogger, settings); + return check(&errorLogger, settings); } CheckUnusedFunctions::FunctionDecl::FunctionDecl(const Function *f) diff --git a/lib/checkunusedfunctions.h b/lib/checkunusedfunctions.h index 1b404b3b6..ea17bc38a 100644 --- a/lib/checkunusedfunctions.h +++ b/lib/checkunusedfunctions.h @@ -55,13 +55,14 @@ public: // * What functions are declared void parseTokens(const Tokenizer &tokenizer, const char FileName[], const Settings *settings, bool clear=true); - void check(ErrorLogger * const errorLogger, const Settings& settings); + // Return true if an error is reported. + bool check(ErrorLogger * const errorLogger, const Settings& settings); /** @brief Parse current TU and extract file info */ Check::FileInfo *getFileInfo(const Tokenizer *tokenizer, const Settings *settings) const; /** @brief Analyse all file infos for all TU */ - void analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger); + bool analyseWholeProgram(const std::list &fileInfo, const Settings& settings, ErrorLogger &errorLogger); static CheckUnusedFunctions instance; diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 47778a90c..6ffe291df 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -809,11 +809,13 @@ void CppCheck::getErrorMessages() Preprocessor::getErrorMessages(this, &s); } -void CppCheck::analyseWholeProgram() +bool CppCheck::analyseWholeProgram() { + bool errors = false; // Analyse the tokens for (std::list::const_iterator it = Check::instances().begin(); it != Check::instances().end(); ++it) - (*it)->analyseWholeProgram(fileInfo, _settings, *this); + errors |= (*it)->analyseWholeProgram(fileInfo, _settings, *this); + return errors; } void CppCheck::analyseWholeProgram(const std::string &buildDir, const std::map &files) diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 81d9bb8b0..634afaf64 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -131,9 +131,10 @@ public: /** Analyse whole program, run this after all TUs has been scanned. * This is deprecated and the plan is to remove this when - * .analyzeinfo is good enough + * .analyzeinfo is good enough. + * Return true if an error is reported. */ - void analyseWholeProgram(); + bool analyseWholeProgram(); /** analyse whole program use .analyzeinfo files */ void analyseWholeProgram(const std::string &buildDir, const std::map &files); diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 1b6c1ff76..6ea1f46b6 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -161,7 +161,8 @@ private: for (std::map::const_iterator file = files.begin(); file != files.end(); ++file) { exitCode |= cppCheck.check(file->first, file->second); } - cppCheck.analyseWholeProgram(); + if (cppCheck.analyseWholeProgram()) + exitCode |= settings.exitCode; reportSuppressions(settings, files); diff --git a/test/testunusedfunctions.cpp b/test/testunusedfunctions.cpp index 85c8e1ace..c60fcc856 100644 --- a/test/testunusedfunctions.cpp +++ b/test/testunusedfunctions.cpp @@ -77,7 +77,11 @@ private: // Check for unused functions.. CheckUnusedFunctions checkUnusedFunctions(&tokenizer, &settings, this); checkUnusedFunctions.parseTokens(tokenizer, "someFile.c", &settings); - checkUnusedFunctions.check(this, settings); + // check() returns error if and only if errout is not empty. + if (checkUnusedFunctions.check(this, settings)) + ASSERT(errout.str() != ""); + else + ASSERT_EQUALS("", errout.str()); } void incondition() {