diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index 9510842a0..7d64d93f8 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -22,6 +22,7 @@ #include "config.h" #include "cppcheck.h" #include "cppcheckexecutor.h" +#include "errorlogger.h" #include "errortypes.h" #include "importproject.h" #include "settings.h" @@ -48,6 +49,9 @@ #include #include #include + +// required for FD_ZERO +using std::memset; #endif #ifdef THREADING_MODEL_WIN @@ -55,21 +59,9 @@ #include #endif -// required for FD_ZERO -using std::memset; - ThreadExecutor::ThreadExecutor(const std::map &files, Settings &settings, ErrorLogger &errorLogger) : mFiles(files), mSettings(settings), mErrorLogger(errorLogger), mFileCount(0) -{ -#if defined(THREADING_MODEL_FORK) - mWpipe = 0; -#elif defined(THREADING_MODEL_WIN) - mProcessedFiles = 0; - mTotalFiles = 0; - mProcessedSize = 0; - mTotalFileSize = 0; -#endif -} +{} ThreadExecutor::~ThreadExecutor() {} @@ -80,22 +72,75 @@ void ThreadExecutor::addFileContent(const std::string &path, const std::string & mFileContents[path] = content; } -void ThreadExecutor::reportErr(const ErrorMessage &msg) -{ - report(msg, MessageType::REPORT_ERROR); -} - -void ThreadExecutor::reportInfo(const ErrorMessage &msg) -{ - report(msg, MessageType::REPORT_INFO); -} - /////////////////////////////////////////////////////////////////////////////// ////// This code is for platforms that support fork() only //////////////////// /////////////////////////////////////////////////////////////////////////////// #if defined(THREADING_MODEL_FORK) +class PipeWriter : public ErrorLogger { +public: + enum PipeSignal {REPORT_OUT='1',REPORT_ERROR='2', REPORT_INFO='3', REPORT_VERIFICATION='4', CHILD_END='5'}; + + explicit PipeWriter(int pipe) : mWpipe(pipe) {} + + void reportOut(const std::string &outmsg, Color c) override { + writeToPipe(REPORT_OUT, ::toString(c) + outmsg + ::toString(Color::Reset)); + } + + void reportErr(const ErrorMessage &msg) override { + report(msg, MessageType::REPORT_ERROR); + } + + void reportInfo(const ErrorMessage &msg) override { + report(msg, MessageType::REPORT_INFO); + } + + void bughuntingReport(const std::string &str) override { + writeToPipe(REPORT_VERIFICATION, str); + } + + void writeEnd(const std::string& str) { + writeToPipe(CHILD_END, str); + } + +private: + enum class MessageType {REPORT_ERROR, REPORT_INFO}; + + void report(const ErrorMessage &msg, MessageType msgType) { + PipeSignal pipeSignal; + switch (msgType) { + case MessageType::REPORT_ERROR: + pipeSignal = REPORT_ERROR; + break; + case MessageType::REPORT_INFO: + pipeSignal = REPORT_INFO; + break; + } + + writeToPipe(pipeSignal, msg.serialize()); + } + + void writeToPipe(PipeSignal type, const std::string &data) + { + unsigned int len = static_cast(data.length() + 1); + char *out = new char[len + 1 + sizeof(len)]; + out[0] = static_cast(type); + std::memcpy(&(out[1]), &len, sizeof(len)); + std::memcpy(&(out[1+sizeof(len)]), data.c_str(), len); + if (write(mWpipe, out, len + 1 + sizeof(len)) <= 0) { + delete[] out; + out = nullptr; + std::cerr << "#### ThreadExecutor::writeToPipe, Failed to write to pipe" << std::endl; + std::exit(EXIT_FAILURE); + } + + delete[] out; + } + + const int mWpipe; +}; + int ThreadExecutor::handleRead(int rpipe, unsigned int &result) { char type = 0; @@ -108,7 +153,7 @@ int ThreadExecutor::handleRead(int rpipe, unsigned int &result) return -1; } - if (type != REPORT_OUT && type != REPORT_ERROR && type != REPORT_INFO && type != CHILD_END) { + if (type != PipeWriter::REPORT_OUT && type != PipeWriter::REPORT_ERROR && type != PipeWriter::REPORT_INFO && type != PipeWriter::CHILD_END) { std::cerr << "#### ThreadExecutor::handleRead error, type was:" << type << std::endl; std::exit(EXIT_FAILURE); } @@ -129,9 +174,9 @@ int ThreadExecutor::handleRead(int rpipe, unsigned int &result) } buf[readIntoBuf] = 0; - if (type == REPORT_OUT) { + if (type == PipeWriter::REPORT_OUT) { mErrorLogger.reportOut(buf); - } else if (type == REPORT_ERROR || type == REPORT_INFO) { + } else if (type == PipeWriter::REPORT_ERROR || type == PipeWriter::REPORT_INFO) { ErrorMessage msg; try { msg.deserialize(buf); @@ -145,13 +190,13 @@ int ThreadExecutor::handleRead(int rpipe, unsigned int &result) std::string errmsg = msg.toString(mSettings.verbose); if (std::find(mErrorList.begin(), mErrorList.end(), errmsg) == mErrorList.end()) { mErrorList.emplace_back(errmsg); - if (type == REPORT_ERROR) + if (type == PipeWriter::REPORT_ERROR) mErrorLogger.reportErr(msg); else mErrorLogger.reportInfo(msg); } } - } else if (type == CHILD_END) { + } else if (type == PipeWriter::CHILD_END) { std::istringstream iss(buf); unsigned int fileResult = 0; iss >> fileResult; @@ -231,9 +276,9 @@ unsigned int ThreadExecutor::check() prctl(PR_SET_PDEATHSIG, SIGHUP); #endif close(pipes[0]); - mWpipe = pipes[1]; - CppCheck fileChecker(*this, false, CppCheckExecutor::executeCommand); + PipeWriter pipewriter(pipes[1]); + CppCheck fileChecker(pipewriter, false, CppCheckExecutor::executeCommand); fileChecker.settings() = mSettings; unsigned int resultOfCheck = 0; @@ -249,7 +294,7 @@ unsigned int ThreadExecutor::check() std::ostringstream oss; oss << resultOfCheck; - writeToPipe(CHILD_END, oss.str()); + pipewriter.writeEnd(oss.str()); std::exit(EXIT_SUCCESS); } @@ -341,48 +386,6 @@ unsigned int ThreadExecutor::check() return result; } -void ThreadExecutor::writeToPipe(PipeSignal type, const std::string &data) -{ - unsigned int len = static_cast(data.length() + 1); - char *out = new char[len + 1 + sizeof(len)]; - out[0] = static_cast(type); - std::memcpy(&(out[1]), &len, sizeof(len)); - std::memcpy(&(out[1+sizeof(len)]), data.c_str(), len); - if (write(mWpipe, out, len + 1 + sizeof(len)) <= 0) { - delete[] out; - out = nullptr; - std::cerr << "#### ThreadExecutor::writeToPipe, Failed to write to pipe" << std::endl; - std::exit(EXIT_FAILURE); - } - - delete[] out; -} - -void ThreadExecutor::reportOut(const std::string &outmsg, Color c) -{ - writeToPipe(REPORT_OUT, ::toString(c) + outmsg + ::toString(Color::Reset)); -} - -void ThreadExecutor::bughuntingReport(const std::string &str) -{ - writeToPipe(REPORT_VERIFICATION, str); -} - -void ThreadExecutor::report(const ErrorMessage &msg, MessageType msgType) -{ - PipeSignal pipeSignal; - switch (msgType) { - case MessageType::REPORT_ERROR: - pipeSignal = REPORT_ERROR; - break; - case MessageType::REPORT_INFO: - pipeSignal = REPORT_INFO; - break; - } - - writeToPipe(pipeSignal, msg.serialize()); -} - void ThreadExecutor::reportInternalChildErr(const std::string &childname, const std::string &msg) { std::list locations; @@ -400,25 +403,101 @@ void ThreadExecutor::reportInternalChildErr(const std::string &childname, const #elif defined(THREADING_MODEL_WIN) +class ThreadExecutor::LogWriter : public ErrorLogger +{ +public: + LogWriter(ThreadExecutor &threadExecutor) + : mThreadExecutor(threadExecutor), mProcessedFiles(0), mTotalFiles(0), mProcessedSize(0), mTotalFileSize(0) { + + mItNextFile = threadExecutor.mFiles.begin(); + mItNextFileSettings = threadExecutor.mSettings.project.fileSettings.begin(); + + mTotalFiles = threadExecutor.mFiles.size() + threadExecutor.mSettings.project.fileSettings.size(); + for (std::map::const_iterator i = threadExecutor.mFiles.begin(); i != threadExecutor.mFiles.end(); ++i) { + mTotalFileSize += i->second; + } + } + + void reportOut(const std::string &outmsg, Color c) override + { + std::lock_guard lg(mReportSync); + + mThreadExecutor.mErrorLogger.reportOut(outmsg, c); + } + + void reportErr(const ErrorMessage &msg) override { + report(msg, MessageType::REPORT_ERROR); + } + + void reportInfo(const ErrorMessage &msg) override { + report(msg, MessageType::REPORT_INFO); + } + + void bughuntingReport(const std::string &str) override + { + std::lock_guard lg(mReportSync); + mThreadExecutor.mErrorLogger.bughuntingReport(str); + } + + ThreadExecutor &mThreadExecutor; + + std::map::const_iterator mItNextFile; + std::list::const_iterator mItNextFileSettings; + + std::size_t mProcessedFiles; + std::size_t mTotalFiles; + std::size_t mProcessedSize; + std::size_t mTotalFileSize; + + std::mutex mFileSync; + std::mutex mErrorSync; + std::mutex mReportSync; + +private: + enum class MessageType {REPORT_ERROR, REPORT_INFO}; + + void report(const ErrorMessage &msg, MessageType msgType) + { + if (mThreadExecutor.mSettings.nomsg.isSuppressed(msg.toSuppressionsErrorMessage())) + return; + + // Alert only about unique errors + bool reportError = false; + const std::string errmsg = msg.toString(mThreadExecutor.mSettings.verbose); + + { + std::lock_guard lg(mErrorSync); + if (std::find(mThreadExecutor.mErrorList.begin(), mThreadExecutor.mErrorList.end(), errmsg) == mThreadExecutor.mErrorList.end()) { + mThreadExecutor.mErrorList.emplace_back(errmsg); + reportError = true; + } + } + + if (reportError) { + std::lock_guard lg(mReportSync); + + switch (msgType) { + case MessageType::REPORT_ERROR: + mThreadExecutor.mErrorLogger.reportErr(msg); + break; + case MessageType::REPORT_INFO: + mThreadExecutor.mErrorLogger.reportInfo(msg); + break; + } + } + } +}; + unsigned int ThreadExecutor::check() { std::vector> threadFutures; threadFutures.reserve(mSettings.jobs); - mItNextFile = mFiles.begin(); - mItNextFileSettings = mSettings.project.fileSettings.begin(); - - mProcessedFiles = 0; - mProcessedSize = 0; - mTotalFiles = mFiles.size() + mSettings.project.fileSettings.size(); - mTotalFileSize = 0; - for (std::map::const_iterator i = mFiles.begin(); i != mFiles.end(); ++i) { - mTotalFileSize += i->second; - } + LogWriter logwriter(*this); for (unsigned int i = 0; i < mSettings.jobs; ++i) { try { - threadFutures.emplace_back(std::async(std::launch::async, threadProc, this)); + threadFutures.emplace_back(std::async(std::launch::async, threadProc, &logwriter)); } catch (const std::system_error &e) { std::cerr << "#### ThreadExecutor::check exception :" << e.what() << std::endl; @@ -431,35 +510,35 @@ unsigned int ThreadExecutor::check() }); } -unsigned int __stdcall ThreadExecutor::threadProc(ThreadExecutor* threadExecutor) +unsigned int __stdcall ThreadExecutor::threadProc(LogWriter* logWriter) { unsigned int result = 0; - std::map::const_iterator &itFile = threadExecutor->mItNextFile; - std::list::const_iterator &itFileSettings = threadExecutor->mItNextFileSettings; + std::map::const_iterator &itFile = logWriter->mItNextFile; + std::list::const_iterator &itFileSettings = logWriter->mItNextFileSettings; // guard static members of CppCheck against concurrent access - threadExecutor->mFileSync.lock(); + logWriter->mFileSync.lock(); for (;;) { - if (itFile == threadExecutor->mFiles.end() && itFileSettings == threadExecutor->mSettings.project.fileSettings.end()) { - threadExecutor->mFileSync.unlock(); + if (itFile == logWriter->mThreadExecutor.mFiles.end() && itFileSettings == logWriter->mThreadExecutor.mSettings.project.fileSettings.end()) { + logWriter->mFileSync.unlock(); break; } - CppCheck fileChecker(*threadExecutor, false, CppCheckExecutor::executeCommand); - fileChecker.settings() = threadExecutor->mSettings; + CppCheck fileChecker(*logWriter, false, CppCheckExecutor::executeCommand); + fileChecker.settings() = logWriter->mThreadExecutor.mSettings; std::size_t fileSize = 0; - if (itFile != threadExecutor->mFiles.end()) { + if (itFile != logWriter->mThreadExecutor.mFiles.end()) { const std::string &file = itFile->first; fileSize = itFile->second; ++itFile; - threadExecutor->mFileSync.unlock(); + logWriter->mFileSync.unlock(); - const std::map::const_iterator fileContent = threadExecutor->mFileContents.find(file); - if (fileContent != threadExecutor->mFileContents.end()) { + const std::map::const_iterator fileContent = logWriter->mThreadExecutor.mFileContents.find(file); + if (fileContent != logWriter->mThreadExecutor.mFileContents.end()) { // File content was given as a string result += fileChecker.check(file, fileContent->second); } else { @@ -469,64 +548,21 @@ unsigned int __stdcall ThreadExecutor::threadProc(ThreadExecutor* threadExecutor } else { // file settings.. const ImportProject::FileSettings &fs = *itFileSettings; ++itFileSettings; - threadExecutor->mFileSync.unlock(); + logWriter->mFileSync.unlock(); result += fileChecker.check(fs); - if (threadExecutor->mSettings.clangTidy) + if (logWriter->mThreadExecutor.mSettings.clangTidy) fileChecker.analyseClangTidy(fs); } - threadExecutor->mFileSync.lock(); + logWriter->mFileSync.lock(); - threadExecutor->mProcessedSize += fileSize; - threadExecutor->mProcessedFiles++; - if (!threadExecutor->mSettings.quiet) { - std::lock_guard lg(threadExecutor->mReportSync); - CppCheckExecutor::reportStatus(threadExecutor->mProcessedFiles, threadExecutor->mTotalFiles, threadExecutor->mProcessedSize, threadExecutor->mTotalFileSize); + logWriter->mProcessedSize += fileSize; + logWriter->mProcessedFiles++; + if (!logWriter->mThreadExecutor.mSettings.quiet) { + std::lock_guard lg(logWriter->mReportSync); + CppCheckExecutor::reportStatus(logWriter->mProcessedFiles, logWriter->mTotalFiles, logWriter->mProcessedSize, logWriter->mTotalFileSize); } } return result; } - -void ThreadExecutor::reportOut(const std::string &outmsg, Color c) -{ - std::lock_guard lg(mReportSync); - - mErrorLogger.reportOut(outmsg, c); -} - -void ThreadExecutor::bughuntingReport(const std::string & /*str*/) -{ - // TODO -} - -void ThreadExecutor::report(const ErrorMessage &msg, MessageType msgType) -{ - if (mSettings.nomsg.isSuppressed(msg.toSuppressionsErrorMessage())) - return; - - // Alert only about unique errors - bool reportError = false; - const std::string errmsg = msg.toString(mSettings.verbose); - - { - std::lock_guard lg(mErrorSync); - if (std::find(mErrorList.begin(), mErrorList.end(), errmsg) == mErrorList.end()) { - mErrorList.emplace_back(errmsg); - reportError = true; - } - } - - if (reportError) { - std::lock_guard lg(mReportSync); - - switch (msgType) { - case MessageType::REPORT_ERROR: - mErrorLogger.reportErr(msg); - break; - case MessageType::REPORT_INFO: - mErrorLogger.reportInfo(msg); - break; - } - } -} #endif diff --git a/cli/threadexecutor.h b/cli/threadexecutor.h index 640950186..0c455635c 100644 --- a/cli/threadexecutor.h +++ b/cli/threadexecutor.h @@ -19,9 +19,6 @@ #ifndef THREADEXECUTOR_H #define THREADEXECUTOR_H -#include "color.h" -#include "errorlogger.h" - #include #include #include @@ -31,13 +28,12 @@ #define THREADING_MODEL_FORK #elif defined(_WIN32) #define THREADING_MODEL_WIN -#include "importproject.h" -#include #else -#error "No threading moodel defined" +#error "No threading model defined" #endif class Settings; +class ErrorLogger; /// @addtogroup CLI /// @{ @@ -46,19 +42,14 @@ class Settings; * This class will take a list of filenames and settings and check then * all files using threads. */ -class ThreadExecutor : public ErrorLogger { +class ThreadExecutor { public: ThreadExecutor(const std::map &files, Settings &settings, ErrorLogger &errorLogger); ThreadExecutor(const ThreadExecutor &) = delete; - ~ThreadExecutor() override; + ~ThreadExecutor(); void operator=(const ThreadExecutor &) = delete; unsigned int check(); - void reportOut(const std::string &outmsg, Color c) override; - void reportErr(const ErrorMessage &msg) override; - void reportInfo(const ErrorMessage &msg) override; - void bughuntingReport(const std::string &str) override; - /** * @brief Add content to a file, to be used in unit testing. * @@ -69,21 +60,16 @@ public: void addFileContent(const std::string &path, const std::string &content); private: - enum class MessageType {REPORT_ERROR, REPORT_INFO}; - const std::map &mFiles; Settings &mSettings; ErrorLogger &mErrorLogger; unsigned int mFileCount; - - void report(const ErrorMessage &msg, MessageType msgType); - -#if defined(THREADING_MODEL_FORK) + std::list mErrorList; /** @brief Key is file name, and value is the content of the file */ std::map mFileContents; -private: - enum PipeSignal {REPORT_OUT='1',REPORT_ERROR='2', REPORT_INFO='3', REPORT_VERIFICATION='4', CHILD_END='5'}; + +#if defined(THREADING_MODEL_FORK) /** * Read from the pipe, parse and handle what ever is in there. @@ -92,13 +78,6 @@ private: * 1 if we did read something */ int handleRead(int rpipe, unsigned int &result); - void writeToPipe(PipeSignal type, const std::string &data); - /** - * Write end of status pipe, different for each child. - * Not used in master process. - */ - std::list mErrorList; - int mWpipe; /** * @brief Check load average condition @@ -113,32 +92,12 @@ private: */ void reportInternalChildErr(const std::string &childname, const std::string &msg); -public: - /** - * @return true if support for threads exist. - */ - static bool isEnabled() { - return true; - } - #elif defined(THREADING_MODEL_WIN) -private: - std::map mFileContents; - std::map::const_iterator mItNextFile; - std::list::const_iterator mItNextFileSettings; - std::size_t mProcessedFiles; - std::size_t mTotalFiles; - std::size_t mProcessedSize; - std::size_t mTotalFileSize; - std::mutex mFileSync; + class LogWriter; + static unsigned __stdcall threadProc(LogWriter *threadExecutor); - std::list mErrorList; - std::mutex mErrorSync; - - std::mutex mReportSync; - - static unsigned __stdcall threadProc(ThreadExecutor *threadExecutor); +#endif public: /** @@ -147,15 +106,7 @@ public: static bool isEnabled() { return true; } -#else -public: - /** - * @return true if support for threads exist. - */ - static bool isEnabled() { - return false; - } -#endif + }; /// @}