From 903769a388b8c76bcc0836fd4ab2e7227f3bcf6e Mon Sep 17 00:00:00 2001 From: Kimmo Varis Date: Sat, 23 Apr 2011 15:20:55 +0300 Subject: [PATCH 1/2] CLI: Give files to Cppcheck class one at a time. When doing single-threaded checking give checked files to Cppcheck class one file at a time. Like GUI and multithreaded checking already do. This unifies how we call Cppcheck class from different clients. --- cli/cppcheckexecutor.cpp | 14 ++++++++++---- cli/cppcheckexecutor.h | 6 ++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 32607e947..74d32ce25 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -114,7 +114,7 @@ bool CppCheckExecutor::parseFromArgs(CppCheck *cppcheck, int argc, const char* c { std::vector::iterator iter; for (iter = filenames.begin(); iter != filenames.end(); ++iter) - cppcheck->addFile(*iter); + _filenames.push_back(*iter); return true; } @@ -146,7 +146,14 @@ int CppCheckExecutor::check(int argc, const char* const argv[]) if (_settings._jobs == 1) { // Single process - returnValue = cppCheck.check(); + for (unsigned int c = 0; c < _filenames.size(); c++) + { + cppCheck.addFile(_filenames[c]); + returnValue += cppCheck.check(); + cppCheck.clearFiles(); + + reportStatus(c + 1, _filenames.size()); + } } else if (!ThreadExecutor::isEnabled()) { @@ -155,9 +162,8 @@ int CppCheckExecutor::check(int argc, const char* const argv[]) else { // Multiple processes - const std::vector &filenames = cppCheck.filenames(); Settings &settings = cppCheck.settings(); - ThreadExecutor executor(filenames, settings, *this); + ThreadExecutor executor(_filenames, settings, *this); returnValue = executor.check(); } diff --git a/cli/cppcheckexecutor.h b/cli/cppcheckexecutor.h index 339d16959..3a0828024 100644 --- a/cli/cppcheckexecutor.h +++ b/cli/cppcheckexecutor.h @@ -22,6 +22,7 @@ #include "errorlogger.h" #include "settings.h" #include +#include class CppCheck; @@ -107,6 +108,11 @@ private: * Has --errorlist been given? */ bool errorlist; + + /** + * List of files to check. + */ + std::vector _filenames; }; #endif // CPPCHECKEXECUTOR_H From f240574107da703f176042eb77ace5434832ff00 Mon Sep 17 00:00:00 2001 From: Kimmo Varis Date: Sun, 24 Apr 2011 19:10:25 +0300 Subject: [PATCH 2/2] Modify the Cppcheck class to check one file at a time. Unify usage and API of CppCheck class. Allow only one file checked at a time, instead of list of files. Clients can then handle file lists more naturally and as they see fit. Also clients have better knowledge of how checking status should be handled. The single-threaded CLI checking was only one using the file list. Other clients were giving files (to list) one file at a time. --- cli/cppcheckexecutor.cpp | 5 +- cli/threadexecutor.cpp | 6 +- gui/checkthread.cpp | 4 +- lib/cppcheck.cpp | 250 +++++++++++++++++--------------------- lib/cppcheck.h | 59 +++++---- test/testsuppressions.cpp | 6 +- 6 files changed, 150 insertions(+), 180 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 74d32ce25..637406de5 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -148,10 +148,7 @@ int CppCheckExecutor::check(int argc, const char* const argv[]) // Single process for (unsigned int c = 0; c < _filenames.size(); c++) { - cppCheck.addFile(_filenames[c]); - returnValue += cppCheck.check(); - cppCheck.clearFiles(); - + returnValue += cppCheck.check(_filenames[c]); reportStatus(c + 1, _filenames.size()); } } diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index 46527a9da..b5daae65c 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -177,19 +177,19 @@ unsigned int ThreadExecutor::check() CppCheck fileChecker(*this, false); fileChecker.settings(_settings); + unsigned int resultOfCheck = 0; if (_fileContents.size() > 0 && _fileContents.find(_filenames[i]) != _fileContents.end()) { // File content was given as a string - fileChecker.addFile(_filenames[i], _fileContents[ _filenames[i] ]); + resultOfCheck = fileChecker.check(_filenames[i], _fileContents[ _filenames[i] ]); } else { // Read file from a file - fileChecker.addFile(_filenames[i]); + resultOfCheck = fileChecker.check(_filenames[i]); } - unsigned int resultOfCheck = fileChecker.check(); std::ostringstream oss; oss << resultOfCheck; writeToPipe('3', oss.str()); diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index 5ae40a3ef..b06627396 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -50,9 +50,7 @@ void CheckThread::run() while (!file.isEmpty() && mState == Running) { qDebug() << "Checking file" << file; - mCppcheck.addFile(file.toStdString()); - mCppcheck.check(); - mCppcheck.clearFiles(); + mCppcheck.check(file.toStdString()); emit FileChecked(file); if (mState == Running) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index dd8ad23d0..13d34bcc9 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -54,155 +54,138 @@ void CppCheck::settings(const Settings ¤tSettings) _settings = currentSettings; } -void CppCheck::addFile(const std::string &filepath) -{ - _filenames.push_back(Path::fromNativeSeparators(filepath)); -} - -void CppCheck::addFile(const std::string &path, const std::string &content) -{ - _filenames.push_back(Path::fromNativeSeparators(path)); - _fileContents[ path ] = content; -} - -void CppCheck::clearFiles() -{ - _filenames.clear(); - _fileContents.clear(); -} - const char * CppCheck::version() { return "1.48"; } -unsigned int CppCheck::check() +unsigned int CppCheck::check(const std::string &path) +{ + _filename = path; + return processFile(); +} + +unsigned int CppCheck::check(const std::string &path, const std::string &content) +{ + _filename = path; + _fileContent = content; + const unsigned int retval = processFile(); + _fileContent.clear(); + return retval; +} + +unsigned int CppCheck::processFile() { exitcode = 0; - std::sort(_filenames.begin(), _filenames.end()); - // TODO: Should this be moved out to its own function so all the files can be // analysed before any files are checked? if (_settings.test_2_pass && _settings._jobs == 1) { - for (unsigned int c = 0; c < _filenames.size(); c++) - { - const std::string fname = _filenames[c]; - if (_settings.terminated()) - break; + const std::string printname = Path::toNativeSeparators(_filename); + reportOut("Analysing " + printname + "..."); - std::string fixedname = Path::toNativeSeparators(fname); - reportOut("Analysing " + fixedname + ".."); - - std::ifstream f(fname.c_str()); - analyseFile(f, fname); - } + std::ifstream f(_filename.c_str()); + analyseFile(f, _filename); } - for (unsigned int c = 0; c < _filenames.size(); c++) + _errout.str(""); + + if (_settings.terminated()) + return exitcode; + + if (_settings._errorsOnly == false) { - _errout.str(""); - const std::string fname = _filenames[c]; - - if (_settings.terminated()) - break; - - if (_settings._errorsOnly == false) - { - std::string fixedpath(fname); - fixedpath = Path::simplifyPath(fixedpath.c_str()); - fixedpath = Path::toNativeSeparators(fixedpath); - _errorLogger.reportOut(std::string("Checking ") + fixedpath + std::string("...")); - } - - try - { - Preprocessor preprocessor(&_settings, this); - std::list configurations; - std::string filedata = ""; - - if ((!_fileContents.empty()) && (_fileContents.find(_filenames[c]) != _fileContents.end())) - { - // File content was given as a string - std::istringstream iss(_fileContents[ _filenames[c] ]); - preprocessor.preprocess(iss, filedata, configurations, fname, _settings._includePaths); - } - else - { - // Only file name was given, read the content from file - std::ifstream fin(fname.c_str()); - Timer t("Preprocessor::preprocess", _settings._showtime, &S_timerResults); - preprocessor.preprocess(fin, filedata, configurations, fname, _settings._includePaths); - } - - _settings.ifcfg = bool(configurations.size() > 1); - - if (!_settings.userDefines.empty()) - { - configurations.clear(); - configurations.push_back(_settings.userDefines); - } - - int checkCount = 0; - for (std::list::const_iterator it = configurations.begin(); it != configurations.end(); ++it) - { - // Check only 12 first configurations, after that bail out, unless --force - // was used. - if (!_settings._force && checkCount > 11) - { - const std::string fixedpath = Path::toNativeSeparators(fname); - ErrorLogger::ErrorMessage::FileLocation location; - location.setfile(fixedpath); - std::list loclist; - loclist.push_back(location); - const std::string msg("Interrupted checking because of too many #ifdef configurations.\n" - "The checking of the file was interrupted because there were too many " - "#ifdef configurations. Checking of all #ifdef configurations can be forced " - "by --force command line option or from GUI preferences. However that may " - "increase the checking time."); - ErrorLogger::ErrorMessage errmsg(loclist, - Severity::information, - msg, - "toomanyconfigs", - false); - _errorLogger.reportErr(errmsg); - break; - } - - cfg = *it; - Timer t("Preprocessor::getcode", _settings._showtime, &S_timerResults); - const std::string codeWithoutCfg = Preprocessor::getcode(filedata, *it, fname, &_settings, &_errorLogger); - t.Stop(); - - // If only errors are printed, print filename after the check - if (_settings._errorsOnly == false && it != configurations.begin()) - { - std::string fixedpath = Path::simplifyPath(fname.c_str()); - fixedpath = Path::toNativeSeparators(fixedpath); - _errorLogger.reportOut(std::string("Checking ") + fixedpath + ": " + cfg + std::string("...")); - } - - std::string appendCode = _settings.append(); - if (!appendCode.empty()) - Preprocessor::preprocessWhitespaces(appendCode); - - checkFile(codeWithoutCfg + appendCode, _filenames[c].c_str()); - ++checkCount; - } - } - catch (std::runtime_error &e) - { - // Exception was thrown when checking this file.. - const std::string fixedpath = Path::toNativeSeparators(fname); - _errorLogger.reportOut("Bailing out from checking " + fixedpath + ": " + e.what()); - } - - reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions(fname)); - - _errorLogger.reportStatus(c + 1, (unsigned int)_filenames.size()); + std::string fixedpath(_filename); + fixedpath = Path::simplifyPath(fixedpath.c_str()); + fixedpath = Path::toNativeSeparators(fixedpath); + _errorLogger.reportOut(std::string("Checking ") + fixedpath + std::string("...")); } + try + { + Preprocessor preprocessor(&_settings, this); + std::list configurations; + std::string filedata = ""; + + if (!_fileContent.empty()) + { + // File content was given as a string + std::istringstream iss(_fileContent); + preprocessor.preprocess(iss, filedata, configurations, _filename, _settings._includePaths); + } + else + { + // Only file name was given, read the content from file + std::ifstream fin(_filename.c_str()); + Timer t("Preprocessor::preprocess", _settings._showtime, &S_timerResults); + preprocessor.preprocess(fin, filedata, configurations, _filename, _settings._includePaths); + } + + _settings.ifcfg = bool(configurations.size() > 1); + + if (!_settings.userDefines.empty()) + { + configurations.clear(); + configurations.push_back(_settings.userDefines); + } + + int checkCount = 0; + for (std::list::const_iterator it = configurations.begin(); it != configurations.end(); ++it) + { + // Check only 12 first configurations, after that bail out, unless --force + // was used. + if (!_settings._force && checkCount > 11) + { + const std::string fixedpath = Path::toNativeSeparators(_filename); + ErrorLogger::ErrorMessage::FileLocation location; + location.setfile(fixedpath); + std::list loclist; + loclist.push_back(location); + const std::string msg("Interrupted checking because of too many #ifdef configurations.\n" + "The checking of the file was interrupted because there were too many " + "#ifdef configurations. Checking of all #ifdef configurations can be forced " + "by --force command line option or from GUI preferences. However that may " + "increase the checking time."); + ErrorLogger::ErrorMessage errmsg(loclist, + Severity::information, + msg, + "toomanyconfigs", + false); + _errorLogger.reportErr(errmsg); + break; + } + + cfg = *it; + Timer t("Preprocessor::getcode", _settings._showtime, &S_timerResults); + const std::string codeWithoutCfg = Preprocessor::getcode(filedata, *it, _filename, &_settings, &_errorLogger); + t.Stop(); + + // If only errors are printed, print filename after the check + if (_settings._errorsOnly == false && it != configurations.begin()) + { + std::string fixedpath = Path::simplifyPath(_filename.c_str()); + fixedpath = Path::toNativeSeparators(fixedpath); + _errorLogger.reportOut(std::string("Checking ") + fixedpath + ": " + cfg + std::string("...")); + } + + std::string appendCode = _settings.append(); + if (!appendCode.empty()) + Preprocessor::preprocessWhitespaces(appendCode); + + checkFile(codeWithoutCfg + appendCode, _filename.c_str()); + ++checkCount; + } + } + catch (std::runtime_error &e) + { + // Exception was thrown when checking this file.. + const std::string fixedpath = Path::toNativeSeparators(_filename); + _errorLogger.reportOut("Bailing out from checking " + fixedpath + ": " + e.what()); + } + + reportUnmatchedSuppressions(_settings.nomsg.getUnmatchedLocalSuppressions(_filename)); + // This generates false positives - especially for libraries const bool verbose_orig = _settings._verbose; _settings._verbose = false; @@ -445,11 +428,6 @@ void CppCheck::reportOut(const std::string &outmsg) _errorLogger.reportOut(outmsg); } -const std::vector &CppCheck::filenames() const -{ - return _filenames; -} - void CppCheck::reportProgress(const std::string &filename, const char stage[], const unsigned int value) { _errorLogger.reportProgress(filename, stage, value); diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 3e02a4e3e..b921bba6a 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -55,7 +55,29 @@ public: * parseFromArgs() or settings() and addFile() before calling this. * @return amount of errors found or 0 if none were found. */ - unsigned int check(); + + /** + * @brief Check the file. + * This function checks one given file for errors. + * @param path Path to the file to check. + * @return amount of errors found or 0 if none were found. + * @note You must set settings before calling this function (by calling + * settings()). + */ + unsigned int check(const std::string &path); + + /** + * @brief Check the file. + * This function checks one "virtual" file. The file is not read from + * the disk but the content is given in @p content. In errors the @p path + * is used as a filename. + * @param path Path to the file to check. + * @param content File content as a string. + * @return amount of errors found or 0 if none were found. + * @note You must set settings before calling this function (by calling + * settings()). + */ + unsigned int check(const std::string &path, const std::string &content); /** * @brief Adjust the settings before doing the check. E.g. show only @@ -71,37 +93,12 @@ public: */ Settings &settings(); - /** - * @brief Add new file to be checked. - * - * @param filepath Relative or absolute path to the file to be checked, - * e.g. "cppcheck.cpp". Note that only source files (.c, .cc or .cpp) - * should be added to the list. Include files are gathered automatically. - */ - void addFile(const std::string &filepath); - - /** - * @brief Add new unreal file to be checked. - * - * @param path File name (used for error reporting). - * @param content If the file would be a real file, this should be - * the content of the file. - */ - void addFile(const std::string &path, const std::string &content); - - /** - * @brief Remove all files added with addFile() and parseFromArgs(). - */ - void clearFiles(); - /** * @brief Returns current version number as a string. * @return version, e.g. "1.38" */ static const char * version(); - const std::vector &filenames() const; - virtual void reportStatus(unsigned int index, unsigned int max); /** @@ -124,6 +121,10 @@ public: void analyseFile(std::istream &f, const std::string &filename); private: + + /** @brief Process one file. */ + unsigned int processFile(); + /** @brief Check file */ void checkFile(const std::string &code, const char FileName[]); @@ -148,13 +149,11 @@ private: std::ostringstream _errout; Settings _settings; bool _useGlobalSuppressions; - std::vector _filenames; + std::string _filename; + std::string _fileContent; void reportProgress(const std::string &filename, const char stage[], const unsigned int value); - /** @brief Key is file name, and value is the content of the file */ - std::map _fileContents; - CheckUnusedFunctions _checkUnusedFunctions; ErrorLogger &_errorLogger; diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index 323651f3f..4631e1d4a 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -55,8 +55,7 @@ private: CppCheck cppCheck(*this, true); cppCheck.settings(settings); - cppCheck.addFile("test.cpp", code); - cppCheck.check(); + cppCheck.check("test.cpp", code); reportUnmatchedSuppressions(cppCheck.settings().nomsg.getUnmatchedGlobalSuppressions()); } @@ -105,8 +104,7 @@ private: CppCheck cppCheck(*this, true); cppCheck.settings(settings); for (int i = 0; names[i] != NULL; ++i) - cppCheck.addFile(names[i], codes[i]); - cppCheck.check(); + cppCheck.check(names[i], codes[i]); reportUnmatchedSuppressions(cppCheck.settings().nomsg.getUnmatchedGlobalSuppressions()); }