diff --git a/Makefile b/Makefile index ca835744d..c0e5675cc 100644 --- a/Makefile +++ b/Makefile @@ -330,7 +330,7 @@ test/teststl.o: test/teststl.cpp lib/tokenize.h lib/checkstl.h lib/check.h lib/t test/testsuite.o: test/testsuite.cpp test/testsuite.h lib/errorlogger.h lib/settings.h test/redirect.h test/options.h $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuite.o test/testsuite.cpp -test/testsuppressions.o: test/testsuppressions.cpp lib/cppcheck.h lib/settings.h lib/errorlogger.h lib/checkunusedfunctions.h lib/check.h lib/token.h lib/tokenize.h test/testsuite.h test/redirect.h +test/testsuppressions.o: test/testsuppressions.cpp lib/cppcheck.h lib/settings.h lib/errorlogger.h lib/checkunusedfunctions.h lib/check.h cli/threadexecutor.h lib/token.h lib/tokenize.h test/testsuite.h test/redirect.h $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuppressions.o test/testsuppressions.cpp test/testsymboldatabase.o: test/testsymboldatabase.cpp test/testsuite.h lib/errorlogger.h lib/settings.h test/redirect.h test/testutils.h lib/tokenize.h lib/token.h lib/symboldatabase.h diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index 7c33d2adb..f9ae2459e 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -20,7 +20,7 @@ #include "cppcheck.h" #include #include -#if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) +#ifdef THREADING_MODEL_FORK #include #include #include @@ -34,8 +34,8 @@ ThreadExecutor::ThreadExecutor(const std::vector &filenames, Settings &settings, ErrorLogger &errorLogger) : _filenames(filenames), _settings(settings), _errorLogger(errorLogger), _fileCount(0) { -#if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) - _pipe[0] = _pipe[1] = 0; +#ifdef THREADING_MODEL_FORK + _wpipe = 0; #endif } @@ -50,15 +50,15 @@ void ThreadExecutor::addFileContent(const std::string &path, const std::string & } /////////////////////////////////////////////////////////////////////////////// -////// This code is for __GNUC__ and __sun only /////////////////////////////// +////// This code is for platforms that support fork() only //////////////////// /////////////////////////////////////////////////////////////////////////////// -#if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) +#ifdef THREADING_MODEL_FORK -int ThreadExecutor::handleRead(unsigned int &result) +int ThreadExecutor::handleRead(int rpipe, unsigned int &result) { char type = 0; - if (read(_pipe[0], &type, 1) <= 0) + if (read(rpipe, &type, 1) <= 0) { if (errno == EAGAIN) return 0; @@ -73,14 +73,14 @@ int ThreadExecutor::handleRead(unsigned int &result) } unsigned int len = 0; - if (read(_pipe[0], &len, sizeof(len)) <= 0) + if (read(rpipe, &len, sizeof(len)) <= 0) { std::cerr << "#### You found a bug from cppcheck.\nThreadExecutor::handleRead error, type was:" << type << std::endl; exit(0); } char *buf = new char[len]; - if (read(_pipe[0], buf, len) <= 0) + if (read(rpipe, buf, len) <= 0) { std::cerr << "#### You found a bug from cppcheck.\nThreadExecutor::handleRead error, type was:" << type << std::endl; exit(0); @@ -134,32 +134,35 @@ unsigned int ThreadExecutor::check() { _fileCount = 0; unsigned int result = 0; - if (pipe(_pipe) == -1) - { - perror("pipe"); - exit(1); - } - int flags = 0; - if ((flags = fcntl(_pipe[0], F_GETFL, 0)) < 0) - { - perror("fcntl"); - exit(1); - } - - if (fcntl(_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) - { - perror("fcntl"); - exit(1); - } - - unsigned int childCount = 0; + std::list rpipes; + std::map childFile; unsigned int i = 0; while (true) { // Start a new child - if (i < _filenames.size() && childCount < _settings._jobs) + if (i < _filenames.size() && rpipes.size() < _settings._jobs) { + int pipes[2]; + if (pipe(pipes) == -1) + { + perror("pipe"); + exit(1); + } + + int flags = 0; + if ((flags = fcntl(pipes[0], F_GETFL, 0)) < 0) + { + perror("fcntl"); + exit(1); + } + + if (fcntl(pipes[0], F_SETFL, flags | O_NONBLOCK) < 0) + { + perror("fcntl"); + exit(1); + } + pid_t pid = fork(); if (pid < 0) { @@ -169,6 +172,9 @@ unsigned int ThreadExecutor::check() } else if (pid == 0) { + close(pipes[0]); + _wpipe = pipes[1]; + CppCheck fileChecker(*this, false); fileChecker.settings(_settings); @@ -190,31 +196,70 @@ unsigned int ThreadExecutor::check() exit(0); } - ++childCount; + close(pipes[1]); + rpipes.push_back(pipes[0]); + childFile[pid] = _filenames[i]; + ++i; } - else if (childCount > 0) + else if (!rpipes.empty()) { - // Wait for child to quit before stating new processes - while (true) + fd_set rfds; + FD_ZERO(&rfds); + for (std::list::const_iterator rp = rpipes.begin(); rp != rpipes.end(); ++rp) + FD_SET(*rp, &rfds); + + int r = select(*std::max_element(rpipes.begin(), rpipes.end()) + 1, &rfds, NULL, NULL, NULL); + + if (r > 0) { - int readRes = handleRead(result); - if (readRes == -1) - break; - else if (readRes == 0) + std::list::iterator rp = rpipes.begin(); + while (rp != rpipes.end()) { - struct timespec duration; - duration.tv_sec = 0; - duration.tv_nsec = 5 * 1000 * 1000; // 5 ms - nanosleep(&duration, NULL); + if (FD_ISSET(*rp, &rfds)) + { + int readRes = handleRead(*rp, result); + if (readRes == -1) + { + close(*rp); + rp = rpipes.erase(rp); + } + else + ++rp; + } + else + ++rp; } } int stat = 0; - waitpid(0, &stat, 0); - --childCount; + pid_t child = waitpid(0, &stat, WNOHANG); + if (child > 0) + { + std::string childname; + std::map::iterator c = childFile.find(child); + if (c != childFile.end()) + { + childname = c->second; + childFile.erase(c); + } + + if (WIFSIGNALED(stat)) + { + std::ostringstream oss; + oss << "Internal error: Child process crashed with signal " << WTERMSIG(stat); + + std::list locations; + locations.push_back(ErrorLogger::ErrorMessage::FileLocation(childname, 0)); + const ErrorLogger::ErrorMessage errmsg(locations, + Severity::error, + oss.str(), + "cppcheckError"); + _errorLogger.reportErr(errmsg); + } + } } - else if (childCount == 0) + else { // All done break; @@ -232,7 +277,7 @@ void ThreadExecutor::writeToPipe(char type, const std::string &data) out[0] = type; std::memcpy(&(out[1]), &len, sizeof(len)); std::memcpy(&(out[1+sizeof(len)]), data.c_str(), len); - if (write(_pipe[1], out, len + 1 + sizeof(len)) <= 0) + if (write(_wpipe, out, len + 1 + sizeof(len)) <= 0) { delete [] out; out = 0; diff --git a/cli/threadexecutor.h b/cli/threadexecutor.h index 309e110ee..bbbabe076 100644 --- a/cli/threadexecutor.h +++ b/cli/threadexecutor.h @@ -25,6 +25,10 @@ #include "settings.h" #include "errorlogger.h" +#if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) +#define THREADING_MODEL_FORK +#endif + /// @addtogroup CLI /// @{ @@ -59,7 +63,7 @@ private: /** @brief Key is file name, and value is the content of the file */ std::map _fileContents; -#if (defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__) +#ifdef THREADING_MODEL_FORK private: /** * Read from the pipe, parse and handle what ever is in there. @@ -67,9 +71,13 @@ private: * 0 if there is nothing in the pipe to be read * 1 if we did read something */ - int handleRead(unsigned int &result); + int handleRead(int rpipe, unsigned int &result); void writeToPipe(char type, const std::string &data); - int _pipe[2]; + /** + * Write end of status pipe, different for each child. + * Not used in master process. + */ + int _wpipe; std::list _errorList; public: /** diff --git a/cppcheck.cppcheck b/cppcheck.cppcheck index 188043b4f..f6a808996 100644 --- a/cppcheck.cppcheck +++ b/cppcheck.cppcheck @@ -13,4 +13,7 @@ + + + diff --git a/gui/about.ui b/gui/about.ui index 8dd7d231b..63c192f1a 100644 --- a/gui/about.ui +++ b/gui/about.ui @@ -1,165 +1,165 @@ - - - About - - - - 0 - 0 - 478 - 300 - - - - About Cppcheck - - - - - - - - - - - - - :/icon.png - - - - - - - Qt::Vertical - - - - 20 - 40 - - - - - - - - - - - - Qt::Horizontal - - - - 40 - 20 - - - - - - - - Version %1 - - - - - - - Cppcheck - A tool for static C/C++ code analysis. - - - true - - - - - - - Copyright © 2007-2010 Daniel Marjamäki and cppcheck team. - - - true - - - - - - - This program is licensed under the terms -of the GNU General Public License version 3 - - - true - - - - - - - Visit Cppcheck homepage at %1 - - - true - - - true - - - - - - - - - - - Qt::Horizontal - - - QDialogButtonBox::Ok - - - - - - - - - - - mButtons - accepted() - About - accept() - - - 248 - 254 - - - 157 - 274 - - - - - mButtons - rejected() - About - reject() - - - 316 - 260 - - - 286 - 274 - - - - - + + + About + + + + 0 + 0 + 478 + 300 + + + + About Cppcheck + + + + + + + + + + + + + :/icon.png + + + + + + + Qt::Vertical + + + + 20 + 40 + + + + + + + + + + + + Qt::Horizontal + + + + 40 + 20 + + + + + + + + Version %1 + + + + + + + Cppcheck - A tool for static C/C++ code analysis. + + + true + + + + + + + Copyright © 2007-2011 Daniel Marjamäki and cppcheck team. + + + true + + + + + + + This program is licensed under the terms +of the GNU General Public License version 3 + + + true + + + + + + + Visit Cppcheck homepage at %1 + + + true + + + true + + + + + + + + + + + Qt::Horizontal + + + QDialogButtonBox::Ok + + + + + + + + + + + mButtons + accepted() + About + accept() + + + 248 + 254 + + + 157 + 274 + + + + + mButtons + rejected() + About + reject() + + + 316 + 260 + + + 286 + 274 + + + + + diff --git a/gui/csvreport.cpp b/gui/csvreport.cpp index 0080f1434..1147d01f3 100644 --- a/gui/csvreport.cpp +++ b/gui/csvreport.cpp @@ -65,7 +65,7 @@ void CsvReport::WriteError(const ErrorItem &error) QString line; const QString file = QDir::toNativeSeparators(error.files[error.files.size() - 1]); line += QString("%1,%2,").arg(file).arg(error.lines[error.lines.size() - 1]); - line += QString("%1,%2").arg(error.severity).arg(error.summary); + line += QString("%1,%2").arg(GuiSeverity::toString(error.severity)).arg(error.summary); mTxtWriter << line << endl; } diff --git a/gui/erroritem.cpp b/gui/erroritem.cpp index a7b09c853..46ab536a9 100644 --- a/gui/erroritem.cpp +++ b/gui/erroritem.cpp @@ -18,6 +18,11 @@ #include "erroritem.h" +ErrorItem::ErrorItem() + : severity(Severity::none) +{ +} + ErrorItem::ErrorItem(const ErrorItem &item) { file = item.file; @@ -42,7 +47,7 @@ ErrorItem::ErrorItem(const ErrorLine &line) QString ErrorItem::ToString() const { - QString str = file + " - " + id + " - " + severity +"\n"; + QString str = file + " - " + id + " - " + GuiSeverity::toString(severity) +"\n"; str += summary + "\n"; str += message + "\n"; for (int i = 0; i < files.size(); i++) diff --git a/gui/erroritem.h b/gui/erroritem.h index c6b36b7c3..cb3db71f9 100644 --- a/gui/erroritem.h +++ b/gui/erroritem.h @@ -22,12 +22,70 @@ #include #include #include +#include "errorlogger.h" class ErrorLine; /// @addtogroup GUI /// @{ + +/** + * @brief GUI versions of severity conversions. + * GUI needs its own versions of conversions since GUI uses Qt's QString + * instead of the std::string used by lib/cli. + */ +class GuiSeverity : Severity +{ +public: + static QString 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"; + }; + return "???"; + } + + static SeverityType fromString(const QString &severity) + { + if (severity.isEmpty()) + 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; + } +}; + /** * @brief A class containing error data for one error. * @@ -39,7 +97,7 @@ class ErrorLine; class ErrorItem { public: - ErrorItem() { } + ErrorItem(); ErrorItem(const ErrorItem &item); ErrorItem(const ErrorLine &line); ~ErrorItem() { } @@ -54,7 +112,7 @@ public: QStringList files; QList lines; QString id; - QString severity; + Severity::SeverityType severity; QString summary; QString message; }; @@ -70,7 +128,7 @@ public: QString file; unsigned int line; QString id; - QString severity; + Severity::SeverityType severity; QString summary; QString message; }; diff --git a/gui/resultstree.cpp b/gui/resultstree.cpp index 5f2630c53..90e785d04 100644 --- a/gui/resultstree.cpp +++ b/gui/resultstree.cpp @@ -190,7 +190,8 @@ QStandardItem *ResultsTree::AddBacktraceFiles(QStandardItem *parent, // Ensure shown path is with native separators const QString file = QDir::toNativeSeparators(item.file); list << CreateNormalItem(file); - list << CreateNormalItem(tr(item.severity.toLatin1())); + const QString severity = GuiSeverity::toString(item.severity); + list << CreateNormalItem(severity.toLatin1()); list << CreateLineNumberItem(QString("%1").arg(item.line)); //TODO message has parameter names so we'll need changes to the core //cppcheck so we can get proper translations @@ -251,24 +252,107 @@ ShowTypes ResultsTree::VariantToShowType(const QVariant &data) return (ShowTypes)value; } -ShowTypes ResultsTree::SeverityToShowType(const QString & severity) +ShowTypes ResultsTree::SeverityToShowType(Severity::SeverityType severity) { - if (severity == "error") + switch (severity) + { + case Severity::none: + return SHOW_NONE; + case Severity::error: return SHOW_ERRORS; - if (severity == "style") + case Severity::style: return SHOW_STYLE; - if (severity == "warning") + case Severity::warning: return SHOW_WARNINGS; - if (severity == "performance") + case Severity::performance: return SHOW_PERFORMANCE; - if (severity == "portability") + case Severity::portability: return SHOW_PORTABILITY; - if (severity == "information") + case Severity::information: return SHOW_INFORMATION; + default: + return SHOW_NONE; + } return SHOW_NONE; } +Severity::SeverityType ResultsTree::ShowTypeToSeverity(ShowTypes type) +{ + switch (type) + { + case SHOW_STYLE: + return Severity::style; + break; + + case SHOW_ERRORS: + return Severity::error; + break; + + case SHOW_WARNINGS: + return Severity::warning; + break; + + case SHOW_PERFORMANCE: + return Severity::performance; + break; + + case SHOW_PORTABILITY: + return Severity::portability; + break; + + case SHOW_INFORMATION: + return Severity::information; + break; + + case SHOW_NONE: + return Severity::none; + break; + } + + return Severity::none; +} + +QString ResultsTree::SeverityToTranslatedString(Severity::SeverityType severity) +{ + switch (severity) + { + case Severity::style: + return tr("style"); + break; + + case Severity::error: + return tr("error"); + break; + + case Severity::warning: + return tr("warning"); + break; + + case Severity::performance: + return tr("performance"); + break; + + case Severity::portability: + return tr("portability"); + break; + + case Severity::information: + return tr("information"); + break; + + case Severity::debug: + return tr("debug"); + break; + + case Severity::none: + return ""; + break; + } + + return ""; +} + QStandardItem *ResultsTree::FindFileItem(const QString &name) { QList list = mModel.findItems(name); @@ -735,21 +819,25 @@ void ResultsTree::CopyPath(QStandardItem *target, bool fullPath) } } -QString ResultsTree::SeverityToIcon(const QString &severity) const +QString ResultsTree::SeverityToIcon(Severity::SeverityType severity) const { - if (severity == "error") + switch (severity) + { + case Severity::error: return ":images/dialog-error.png"; - if (severity == "style") + case Severity::style: return ":images/applications-development.png"; - if (severity == "warning") + case Severity::warning: return ":images/dialog-warning.png"; - if (severity == "portability") + case Severity::portability: return ":images/applications-system.png"; - if (severity == "performance") + case Severity::performance: return ":images/utilities-system-monitor.png"; - if (severity == "information") + case Severity::information: return ":images/dialog-information.png"; - + default: + return ""; + } return ""; } @@ -794,7 +882,7 @@ void ResultsTree::SaveErrors(Report *report, QStandardItem *item) QVariantMap data = userdata.toMap(); ErrorItem item; - item.severity = ShowTypeToString(VariantToShowType(data["severity"])); + item.severity = ShowTypeToSeverity(VariantToShowType(data["severity"])); item.summary = data["summary"].toString(); item.message = data["message"].toString(); item.id = data["id"].toString(); @@ -823,42 +911,6 @@ void ResultsTree::SaveErrors(Report *report, QStandardItem *item) } } -QString ResultsTree::ShowTypeToString(ShowTypes type) -{ - switch (type) - { - case SHOW_STYLE: - return tr("style"); - break; - - case SHOW_ERRORS: - return tr("error"); - break; - - case SHOW_WARNINGS: - return tr("warning"); - break; - - case SHOW_PERFORMANCE: - return tr("performance"); - break; - - case SHOW_PORTABILITY: - return tr("portability"); - break; - - case SHOW_INFORMATION: - return tr("information"); - break; - - case SHOW_NONE: - return ""; - break; - } - - return ""; -} - void ResultsTree::UpdateSettings(bool showFullPath, bool saveFullPath, bool saveAllErrors) diff --git a/gui/resultstree.h b/gui/resultstree.h index 99448261e..cc5f1b03d 100644 --- a/gui/resultstree.h +++ b/gui/resultstree.h @@ -28,6 +28,7 @@ #include #include "common.h" #include "applicationlist.h" +#include "errorlogger.h" // Severity class Report; class ErrorItem; @@ -128,10 +129,10 @@ public: /** * @brief Convert severity string to ShowTypes value - * @param severity Error severity string + * @param severity Error severity * @return Severity converted to ShowTypes value */ - static ShowTypes SeverityToShowType(const QString &severity); + static ShowTypes SeverityToShowType(Severity::SeverityType severity); signals: /** @@ -230,9 +231,9 @@ protected: /** * @brief Convert a severity string to a icon filename * - * @param severity Severity string + * @param severity Severity */ - QString SeverityToIcon(const QString &severity) const; + QString SeverityToIcon(Severity::SeverityType severity) const; /** * @brief Helper function to open an error within target with application* @@ -291,9 +292,16 @@ protected: /** * @brief Convert ShowType to severity string * @param type ShowType to convert - * @return ShowType converted to string + * @return ShowType converted to severity */ - QString ShowTypeToString(ShowTypes type); + Severity::SeverityType ShowTypeToSeverity(ShowTypes type); + + /** + * @brief Convert Severity to translated string for GUI. + * @param type Severity to convert + * @return Severity as translated string + */ + QString SeverityToTranslatedString(Severity::SeverityType severity); /** * @brief Load all settings diff --git a/gui/threadresult.cpp b/gui/threadresult.cpp index 7a92481c4..38e990ba1 100644 --- a/gui/threadresult.cpp +++ b/gui/threadresult.cpp @@ -71,7 +71,7 @@ void ThreadResult::reportErr(const ErrorLogger::ErrorMessage &msg) item.lines = lines; item.summary = QString::fromStdString(msg.shortMessage()); item.message = QString::fromStdString(msg.verboseMessage()); - item.severity = QString::fromStdString(Severity::toString(msg._severity)); + item.severity = msg._severity; if (msg._severity != Severity::debug) emit Error(item); diff --git a/gui/txtreport.cpp b/gui/txtreport.cpp index 20c34d7b7..f7c5ceb86 100644 --- a/gui/txtreport.cpp +++ b/gui/txtreport.cpp @@ -76,7 +76,7 @@ void TxtReport::WriteError(const ErrorItem &error) } } - line += QString("(%1) %2").arg(error.severity).arg(error.summary); + line += QString("(%1) %2").arg(GuiSeverity::toString(error.severity)).arg(error.summary); mTxtWriter << line << endl; } diff --git a/gui/xmlreportv1.cpp b/gui/xmlreportv1.cpp index 1062de07f..bab621f81 100644 --- a/gui/xmlreportv1.cpp +++ b/gui/xmlreportv1.cpp @@ -100,7 +100,9 @@ void XmlReportV1::WriteError(const ErrorItem &error) const QString line = QString::number(error.lines[error.lines.size() - 1]); mXmlWriter->writeAttribute(LineAttribute, line); mXmlWriter->writeAttribute(IdAttribute, error.id); - mXmlWriter->writeAttribute(SeverityAttribute, error.severity); + + // Don't localize severity so we can read these files + mXmlWriter->writeAttribute(SeverityAttribute, GuiSeverity::toString(error.severity)); const QString message = XmlReport::quoteMessage(error.message); mXmlWriter->writeAttribute(MsgAttribute, message); mXmlWriter->writeEndElement(); @@ -165,7 +167,7 @@ ErrorItem XmlReportV1::ReadError(QXmlStreamReader *reader) const int line = attribs.value("", LineAttribute).toString().toUInt(); item.lines.push_back(line); item.id = attribs.value("", IdAttribute).toString(); - item.severity = attribs.value("", SeverityAttribute).toString(); + item.severity = GuiSeverity::fromString(attribs.value("", SeverityAttribute).toString()); // NOTE: This dublicates the message to Summary-field. But since // old XML format doesn't have separate summary and verbose messages diff --git a/gui/xmlreportv2.cpp b/gui/xmlreportv2.cpp index 381d7d9c0..2c5665704 100644 --- a/gui/xmlreportv2.cpp +++ b/gui/xmlreportv2.cpp @@ -110,7 +110,9 @@ void XmlReportV2::WriteError(const ErrorItem &error) mXmlWriter->writeStartElement(ErrorElementName); mXmlWriter->writeAttribute(IdAttribute, error.id); - mXmlWriter->writeAttribute(SeverityAttribute, error.severity); + + // Don't localize severity so we can read these files + mXmlWriter->writeAttribute(SeverityAttribute, GuiSeverity::toString(error.severity)); const QString summary = XmlReport::quoteMessage(error.summary); mXmlWriter->writeAttribute(MsgAttribute, summary); const QString message = XmlReport::quoteMessage(error.message); @@ -196,7 +198,7 @@ ErrorItem XmlReportV2::ReadError(QXmlStreamReader *reader) { QXmlStreamAttributes attribs = reader->attributes(); item.id = attribs.value("", IdAttribute).toString(); - item.severity = attribs.value("", SeverityAttribute).toString(); + item.severity = GuiSeverity::fromString(attribs.value("", SeverityAttribute).toString()); const QString summary = attribs.value("", MsgAttribute).toString(); item.summary = XmlReport::unquoteMessage(summary); const QString message = attribs.value("", VerboseAttribute).toString(); diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index ecd7aac8c..83d9bb068 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -469,6 +469,15 @@ void CheckBufferOverrun::parse_for_body(const Token *tok2, const ArrayInfo &arra if (tok2->str() == "?") break; + if (Token::simpleMatch(tok2, "for (") && Token::simpleMatch(tok2->next()->link(), ") {")) + { + const Token *endpar = tok2->next()->link(); + const Token *startbody = endpar->next(); + const Token *endbody = startbody->link(); + tok2 = endbody; + continue; + } + if (Token::Match(tok2, "if|switch")) { if (bailoutIfSwitch(tok2, arrayInfo.varid)) diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 569f4ed9d..a43c39ea5 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -693,6 +693,103 @@ void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string // ClassCheck: Check that memset is not used on classes //--------------------------------------------------------------------------- +void CheckClass::checkMemsetType(const Token *tok, const std::string &type) +{ + // check for cached message for this type + std::map::const_iterator msg = _memsetClassMessages.find(type); + if (msg != _memsetClassMessages.end()) + { + memsetError(tok, type, msg->second); + return; + } + + // Warn if type is a class or struct that contains any std::* variables + const std::string pattern2(std::string("struct|class ") + type + " :|{"); + const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); + + if (!tstruct) + return; + + // typeKind is either 'struct' or 'class' + const std::string &typeKind = tstruct->str(); + + if (tstruct->tokAt(2)->str() == ":") + { + tstruct = tstruct->tokAt(3); + for (; tstruct; tstruct = tstruct->next()) + { + while (Token::Match(tstruct, "public|private|protected|virtual")) + { + tstruct = tstruct->next(); + } + + // recursively check all parent classes + checkMemsetType(tok, tstruct->str()); + + tstruct = tstruct->next(); + if (tstruct->str() != ",") + break; + } + } + + for (; tstruct; tstruct = tstruct->next()) + { + if (tstruct->str() == "}") + break; + + // struct with function? skip function body.. + if (Token::simpleMatch(tstruct, ") {")) + { + tstruct = tstruct->next()->link(); + if (!tstruct) + break; + } + + // before a statement there must be either: + // * private:|protected:|public: + // * { } ; + if (Token::Match(tstruct, "[;{}]") || + tstruct->str().find(":") != std::string::npos) + { + if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) + memsetError(tok, type, tok->str(), "'std::" + tstruct->strAt(3) + "'", typeKind); + + else if (Token::Match(tstruct->next(), "std :: %type% <")) + { + // backup the type + const std::string typestr(tstruct->strAt(3)); + + // check if it's a pointer variable.. + unsigned int level = 0; + while (0 != (tstruct = tstruct->next())) + { + if (tstruct->str() == "<") + ++level; + else if (tstruct->str() == ">") + { + if (level <= 1) + break; + --level; + } + else if (tstruct->str() == "(") + tstruct = tstruct->link(); + } + + if (!tstruct) + break; + + // found error => report + if (Token::Match(tstruct, "> %var% ;")) + memsetError(tok, type, tok->str(), "'std::" + typestr + "'", typeKind); + } + else if (Token::simpleMatch(tstruct->next(), "virtual")) + memsetError(tok, type, tok->str(), "virtual method", typeKind); + else if (!Token::Match(tstruct->next(), "static|}")) + checkMemsetType(tok, tstruct->next()->str()); + } + } +} + void CheckClass::noMemset() { createSymbolDatabase(); @@ -726,73 +823,20 @@ void CheckClass::noMemset() if (type.empty()) continue; - // Warn if type is a class or struct that contains any std::* variables - const std::string pattern2(std::string("struct|class ") + type + " {"); - const Token *tstruct = Token::findmatch(_tokenizer->tokens(), pattern2.c_str()); - - if (!tstruct) - continue; - - const std::string &typeName = tstruct->str(); - - for (; tstruct; tstruct = tstruct->next()) - { - if (tstruct->str() == "}") - break; - - // struct with function? skip function body.. - if (Token::simpleMatch(tstruct, ") {")) - { - tstruct = tstruct->next()->link(); - if (!tstruct) - break; - } - - // before a statement there must be either: - // * private:|protected:|public: - // * { } ; - if (Token::Match(tstruct, "[;{}]") || - tstruct->str().find(":") != std::string::npos) - { - if (Token::Match(tstruct->next(), "std :: %type% %var% ;")) - memsetError(tok, tok->str(), tstruct->strAt(3), typeName); - - else if (Token::Match(tstruct->next(), "std :: %type% <")) - { - // backup the type - const std::string typestr(tstruct->strAt(3)); - - // check if it's a pointer variable.. - unsigned int level = 0; - while (0 != (tstruct = tstruct->next())) - { - if (tstruct->str() == "<") - ++level; - else if (tstruct->str() == ">") - { - if (level <= 1) - break; - --level; - } - else if (tstruct->str() == "(") - tstruct = tstruct->link(); - } - - if (!tstruct) - break; - - // found error => report - if (Token::Match(tstruct, "> %var% ;")) - memsetError(tok, tok->str(), typestr, typeName); - } - } - } + checkMemsetType(tok, type); } } -void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type) +void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &message) { - reportError(tok, Severity::error, "memsetClass", "Using '" + memfunc + "' on " + type + " that contains a 'std::" + classname + "'"); + reportError(tok, Severity::error, "memsetClass", message); + // cache the message for this type so we don't have to look it up again + _memsetClassMessages[type] = message; +} + +void CheckClass::memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind) +{ + memsetError(tok, type, "Using '" + memfunc + "' on " + typekind + " that contains a " + classname); } //--------------------------------------------------------------------------- diff --git a/lib/checkclass.h b/lib/checkclass.h index 2dd238f83..9cd12f499 100644 --- a/lib/checkclass.h +++ b/lib/checkclass.h @@ -117,7 +117,8 @@ private: void uninitVarError(const Token *tok, const std::string &classname, const std::string &varname); void operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname); void unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname); - void memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type); + void memsetError(const Token *tok, const std::string &type, const std::string &message); + void memsetError(const Token *tok, const std::string &type, const std::string &memfunc, const std::string &classname, const std::string &typekind); void operatorEqReturnError(const Token *tok); void virtualDestructorError(const Token *tok, const std::string &Base, const std::string &Derived); void thisSubtractionError(const Token *tok); @@ -133,7 +134,7 @@ private: c.uninitVarError(0, "classname", "varname"); c.operatorEqVarError(0, "classname", ""); c.unusedPrivateFunctionError(0, "classname", "funcname"); - c.memsetError(0, "memfunc", "classname", "class"); + c.memsetError(0, "type", "memfunc", "classname", "class"); c.operatorEqReturnError(0); //c.virtualDestructorError(0, "Base", "Derived"); c.thisSubtractionError(0); @@ -227,6 +228,10 @@ private: void initializeVarList(const Function &func, std::list &callstack, const Scope *scope, std::vector &usage); bool canNotCopy(const Scope *scope) const; + + // noMemset helpers + void checkMemsetType(const Token *tok, const std::string &type); + std::map _memsetClassMessages; }; /// @} //--------------------------------------------------------------------------- diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 4dd122d1a..8c96509dc 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -75,10 +75,18 @@ bool CheckMemoryLeak::isclass(const Tokenizer *_tokenizer, const Token *tok) con if (tok->isStandardType()) return false; - std::ostringstream pattern; - pattern << "struct " << tok->str(); - if (Token::findmatch(_tokenizer->tokens(), pattern.str().c_str())) - return false; + // return false if the type is a simple struct without member functions + const std::string pattern("struct " + tok->str() + " {"); + const Token *tok2 = Token::findmatch(_tokenizer->tokens(), pattern.c_str()); + if (tok2) + { + while (tok2 && tok2->str() != "}" && tok2->str() != "(") + tok2 = tok2->next(); + + // Simple struct => return false + if (tok2 && tok2->str() == "}") + return false; + } return true; } @@ -1588,17 +1596,19 @@ Token *CheckMemoryLeakInFunction::getcode(const Token *tok, std::listlink(),") (")) { - int tokIdx = matchFirst ? 2 : 3; + const Token *tok2 = tok->next(); + if (tok2->str() == "*") + tok2 = tok2->next(); + tok2 = tok2->next(); - while (Token::Match(tok->tokAt(tokIdx), ". %var%")) - tokIdx += 2; + while (Token::Match(tok2, ". %var%")) + tok2 = tok2->tokAt(2); - if (Token::simpleMatch(tok->tokAt(tokIdx), ") (")) + if (Token::simpleMatch(tok2, ") (")) { - for (const Token *tok2 = tok->tokAt(tokIdx + 2); tok2; tok2 = tok2->next()) + for (; tok2; tok2 = tok2->next()) { if (Token::Match(tok2, "[;{]")) break; @@ -1954,8 +1964,8 @@ void CheckMemoryLeakInFunction::simplifycode(Token *tok) } else { - // remove the "if* ;" - Token::eraseTokens(tok2, tok2->tokAt(3)); + // remove the "if*" + Token::eraseTokens(tok2, tok2->tokAt(2)); } done = false; } diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 6241dceae..4681177d6 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -21,6 +21,7 @@ #include "checknullpointer.h" #include "executionpath.h" #include "mathlib.h" +#include "symboldatabase.h" //--------------------------------------------------------------------------- // Register this check class (by creating a static instance of it) @@ -166,8 +167,12 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown) bool CheckNullPointer::isPointer(const unsigned int varid) { // Check if given variable is a pointer - const Token *tok = Token::findmatch(_tokenizer->tokens(), "%varid%", varid); - tok = tok->tokAt(-2); + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + const Variable *variableInfo = symbolDatabase->getVariableFromVarId(varid); + const Token *tok = variableInfo ? variableInfo->typeStartToken() : NULL; + + if (Token::Match(tok, "%type% %type% * %varid% [;)=]", varid)) + return true; // maybe not a pointer if (!Token::Match(tok, "%type% * %varid% [;)=]", varid)) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 8588e3666..2b9674056 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -24,6 +24,7 @@ #include // std::isupper #include // fabs() +#include //--------------------------------------------------------------------------- // Register this check class (by creating a static instance of it) @@ -306,6 +307,176 @@ void CheckOther::checkRedundantAssignmentInSwitch() } +void CheckOther::checkSwitchCaseFallThrough() +{ + if (!(_settings->_checkCodingStyle && _settings->inconclusive)) + return; + + const char switchPattern[] = "switch ("; + const char breakPattern[] = "break|continue|return|exit|goto"; + + // Find the beginning of a switch. E.g.: + // switch (var) { ... + const Token *tok = Token::findmatch(_tokenizer->tokens(), switchPattern); + while (tok) + { + + // Check the contents of the switch statement + std::stack > ifnest; + std::stack loopnest; + std::stack scopenest; + bool justbreak = true; + bool firstcase = true; + for (const Token *tok2 = tok->tokAt(1)->link()->tokAt(2); tok2; tok2 = tok2->next()) + { + if (Token::Match(tok2, "if (")) + { + tok2 = tok2->tokAt(1)->link()->next(); + if (tok2->link() == NULL) + { + std::ostringstream errmsg; + errmsg << "unmatched if in switch: " << tok2->linenr(); + reportError(_tokenizer->tokens(), Severity::debug, "debug", errmsg.str()); + break; + } + ifnest.push(std::make_pair(tok2->link(), false)); + justbreak = false; + } + else if (Token::Match(tok2, "while (")) + { + tok2 = tok2->tokAt(1)->link()->next(); + // skip over "do { } while ( ) ;" case + if (tok2->str() == "{") + { + if (tok2->link() == NULL) + { + std::ostringstream errmsg; + errmsg << "unmatched while in switch: " << tok2->linenr(); + reportError(_tokenizer->tokens(), Severity::debug, "debug", errmsg.str()); + break; + } + loopnest.push(tok2->link()); + } + justbreak = false; + } + else if (Token::Match(tok2, "do {")) + { + tok2 = tok2->tokAt(1); + if (tok2->link() == NULL) + { + std::ostringstream errmsg; + errmsg << "unmatched do in switch: " << tok2->linenr(); + reportError(_tokenizer->tokens(), Severity::debug, "debug", errmsg.str()); + break; + } + loopnest.push(tok2->link()); + justbreak = false; + } + else if (Token::Match(tok2, "for (")) + { + tok2 = tok2->tokAt(1)->link()->next(); + if (tok2->link() == NULL) + { + std::ostringstream errmsg; + errmsg << "unmatched for in switch: " << tok2->linenr(); + reportError(_tokenizer->tokens(), Severity::debug, "debug", errmsg.str()); + break; + } + loopnest.push(tok2->link()); + justbreak = false; + } + else if (Token::Match(tok2, switchPattern)) + { + // skip over nested switch, we'll come to that soon + tok2 = tok2->tokAt(1)->link()->next()->link(); + } + else if (Token::Match(tok2, breakPattern)) + { + if (loopnest.empty()) + { + justbreak = true; + } + tok2 = Token::findmatch(tok2, ";"); + } + else if (Token::Match(tok2, "case|default")) + { + if (!justbreak && !firstcase) + { + switchCaseFallThrough(tok2); + } + tok2 = Token::findmatch(tok2, ":"); + justbreak = true; + firstcase = false; + } + else if (tok2->str() == "{") + { + scopenest.push(tok2->link()); + } + else if (tok2->str() == "}") + { + if (!ifnest.empty() && tok2 == ifnest.top().first) + { + if (tok2->next()->str() == "else") + { + tok2 = tok2->tokAt(2); + ifnest.pop(); + if (tok2->link() == NULL) + { + std::ostringstream errmsg; + errmsg << "unmatched if in switch: " << tok2->linenr(); + reportError(_tokenizer->tokens(), Severity::debug, "debug", errmsg.str()); + break; + } + ifnest.push(std::make_pair(tok2->link(), justbreak)); + justbreak = false; + } + else + { + justbreak &= ifnest.top().second; + ifnest.pop(); + } + } + else if (!loopnest.empty() && tok2 == loopnest.top()) + { + loopnest.pop(); + } + else if (!scopenest.empty() && tok2 == scopenest.top()) + { + scopenest.pop(); + } + else + { + if (!ifnest.empty() || !loopnest.empty() || !scopenest.empty()) + { + std::ostringstream errmsg; + errmsg << "unexpected end of switch: "; + errmsg << "ifnest=" << ifnest.size(); + if (!ifnest.empty()) + errmsg << "," << ifnest.top().first->linenr(); + errmsg << ", loopnest=" << loopnest.size(); + if (!loopnest.empty()) + errmsg << "," << loopnest.top()->linenr(); + errmsg << ", scopenest=" << scopenest.size(); + if (!scopenest.empty()) + errmsg << "," << scopenest.top()->linenr(); + reportError(_tokenizer->tokens(), Severity::debug, "debug", errmsg.str()); + } + // end of switch block + break; + } + } + else if (tok2->str() != ";") + { + justbreak = false; + } + + } + + tok = Token::findmatch(tok->next(), switchPattern); + } +} + + //--------------------------------------------------------------------------- // int x = 1; // x = x; // <- redundant assignment to self @@ -2811,15 +2982,6 @@ void CheckOther::checkIncorrectStringCompare() incorrectStringCompareError(tok->next(), "substr", tok->str(), tok->tokAt(8)->str()); } } - if (Token::Match(tok, "strncmp ( %any% , %str% , %num% )")) - { - size_t clen = MathLib::toLongNumber(tok->tokAt(6)->str()); - size_t slen = Token::getStrLength(tok->tokAt(4)); - if (clen != slen) - { - incorrectStringCompareError(tok, "strncmp", tok->tokAt(4)->str(), tok->tokAt(6)->str()); - } - } } } @@ -2998,6 +3160,12 @@ void CheckOther::redundantAssignmentInSwitchError(const Token *tok, const std::s "redundantAssignInSwitch", "Redundant assignment of \"" + varname + "\" in switch"); } +void CheckOther::switchCaseFallThrough(const Token *tok) +{ + reportError(tok, Severity::style, + "switchCaseFallThrough", "Switch falls through case without comment"); +} + void CheckOther::selfAssignmentError(const Token *tok, const std::string &varname) { reportError(tok, Severity::warning, diff --git a/lib/checkother.h b/lib/checkother.h index d43c4c413..80605333b 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -90,6 +90,7 @@ public: checkOther.checkIncorrectStringCompare(); checkOther.checkIncrementBoolean(); checkOther.checkComparisonOfBoolWithInt(); + checkOther.checkSwitchCaseFallThrough(); } /** @brief Clarify calculation for ".. a * b ? .." */ @@ -162,6 +163,9 @@ public: /** @brief %Check for assigning to the same variable twice in a switch statement*/ void checkRedundantAssignmentInSwitch(); + /** @brief %Check for switch case fall through without comment */ + void checkSwitchCaseFallThrough(); + /** @brief %Check for assigning a variable to itself*/ void checkSelfAssignment(); @@ -209,6 +213,7 @@ public: void mathfunctionCallError(const Token *tok, const unsigned int numParam = 1); void fflushOnInputStreamError(const Token *tok, const std::string &varname); void redundantAssignmentInSwitchError(const Token *tok, const std::string &varname); + void switchCaseFallThrough(const Token *tok); void selfAssignmentError(const Token *tok, const std::string &varname); void assignmentInAssertError(const Token *tok, const std::string &varname); void incorrectLogicOperatorError(const Token *tok); @@ -247,6 +252,7 @@ public: c.sizeofsizeofError(0); c.sizeofCalculationError(0); c.redundantAssignmentInSwitchError(0, "varname"); + c.switchCaseFallThrough(0); c.selfAssignmentError(0, "varname"); c.assignmentInAssertError(0, "varname"); c.invalidScanfError(0); diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e52a4b79f..3509386ca 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -17,8 +17,8 @@ */ #include "checkstl.h" -#include "token.h" #include "executionpath.h" +#include "symboldatabase.h" #include // Register this check class (by creating a static instance of it) @@ -799,11 +799,13 @@ bool CheckStl::isStlContainer(const Token *tok) if (tok->varId()) { // find where this token is defined - const Token *type = Token::findmatch(_tokenizer->tokens(), "%varid%", tok->varId()); + const Variable *var = _tokenizer->getSymbolDatabase()->getVariableFromVarId(tok->varId()); + + if (!var) + return false; // find where this tokens type starts - while (type->previous() && !Token::Match(type->previous(), "[;{,(]")) - type = type->previous(); + const Token *type = var->typeStartToken(); // ignore "const" if (type->str() == "const") @@ -829,38 +831,27 @@ bool CheckStl::isStlContainer(const Token *tok) void CheckStl::size() { - if (!_settings->_checkCodingStyle || !_settings->inconclusive) + if (!_settings->_checkCodingStyle) return; for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (Token::Match(tok, "%var% . size ( )")) { - if (Token::Match(tok->tokAt(5), "==|!=|> 0")) + // check for comparison to zero + if (Token::Match(tok->tokAt(5), "==|!=|> 0") || + Token::Match(tok->tokAt(-2), "0 ==|!=|<")) { if (isStlContainer(tok)) sizeError(tok); } - else if ((tok->tokAt(5)->str() == ")" || - tok->tokAt(5)->str() == "&&" || - tok->tokAt(5)->str() == "||" || - tok->tokAt(5)->str() == "!") && - (tok->tokAt(-1)->str() == "(" || - tok->tokAt(-1)->str() == "&&" || - tok->tokAt(-1)->str() == "||" || - tok->tokAt(-1)->str() == "!")) + + // check for using as boolean expression + else if ((Token::Match(tok->tokAt(-2), "if|while (") || + Token::Match(tok->tokAt(-3), "if|while ( !")) && + tok->strAt(5) == ")") { - if (tok->tokAt(-1)->str() == "(" && - tok->tokAt(5)->str() == ")") - { - // check for passing size to function call - if (Token::Match(tok->tokAt(-2), "if|while")) - { - if (isStlContainer(tok)) - sizeError(tok); - } - } - else if (isStlContainer(tok)) + if (isStlContainer(tok)) sizeError(tok); } } diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 384562b54..055e551e0 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -35,7 +35,62 @@ class Tokenizer; class Severity { public: - enum SeverityType { none, error, warning, style, performance, portability, information, debug }; + /** + * Message severities. + */ + enum SeverityType + { + /** + * No severity (default value). + */ + none, + /** + * Programming error. + * This indicates severe error like memory leak etc. + * The error is certain. + */ + error, + /** + * Warning. + * Used for dangerous coding style that can cause severe runtime errors. + * For example: forgetting to initialize a member variable in a constructor. + */ + warning, + /** + * Style warning. + * Used for general code cleanup recommendations. Fixing these + * will not fix any bugs but will make the code easier to maintain. + * For example: redundant code, unreachable code, etc. + */ + style, + /** + * Performance warning. + * Not an error as is but suboptimal code and fixing it probably leads + * to faster performance of the compiled code. + */ + performance, + /** + * Portability warning. + * This warning indicates the code is not properly portable for + * different platforms and bitnesses (32/64 bit). If the code is meant + * to compile in different platforms and bitnesses these warnings + * should be fixed. + */ + portability, + /** + * Checking information. + * Information message about the checking (process) itself. These + * messages inform about header files not found etc issues that are + * not errors in the code but something user needs to know. + */ + information, + /** + * Debug message. + * Debug-mode message useful for the developers. + */ + debug + }; + static std::string toString(SeverityType severity) { switch (severity) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 45ed8a425..432cd5f8a 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -303,6 +303,27 @@ static bool hasbom(const std::string &str) } +static bool isFallThroughComment(std::string comment) +{ + // convert comment to lower case without whitespace + std::transform(comment.begin(), comment.end(), comment.begin(), ::tolower); + for (std::string::iterator i = comment.begin(); i != comment.end();) + { + if (::isspace(static_cast(*i))) + i = comment.erase(i); + else + ++i; + } + + return comment.find("fallthr") != std::string::npos || + comment.find("fallsthr") != std::string::npos || + comment.find("fall-thr") != std::string::npos || + comment.find("dropthr") != std::string::npos || + comment.find("passthr") != std::string::npos || + comment.find("nobreak") != std::string::npos || + comment == "fall"; +} + std::string Preprocessor::removeComments(const std::string &str, const std::string &filename, Settings *settings) { // For the error report @@ -314,7 +335,9 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri unsigned int newlines = 0; std::ostringstream code; unsigned char previous = 0; + bool inPreprocessorLine = false; std::vector suppressionIDs; + bool fallThroughComment = false; for (std::string::size_type i = hasbom(str) ? 3U : 0U; i < str.length(); ++i) { @@ -358,6 +381,8 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri // if there has been sequences, add extra newlines.. if (ch == '\n') { + if (previous != '\\') + inPreprocessorLine = false; ++lineno; if (newlines > 0) { @@ -377,10 +402,10 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri i = str.find('\n', i); if (i == std::string::npos) break; + std::string comment(str, commentStart, i - commentStart); if (settings && settings->_inlineSuppressions) { - std::string comment(str, commentStart, i - commentStart); std::istringstream iss(comment); std::string word; iss >> word; @@ -392,6 +417,11 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri } } + if (isFallThroughComment(comment)) + { + fallThroughComment = true; + } + code << "\n"; previous = '\n'; ++lineno; @@ -412,10 +442,15 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri ++lineno; } } + std::string comment(str, commentStart, i - commentStart - 1); + + if (isFallThroughComment(comment)) + { + fallThroughComment = true; + } if (settings && settings->_inlineSuppressions) { - std::string comment(str, commentStart, i - commentStart); std::istringstream iss(comment); std::string word; iss >> word; @@ -427,25 +462,47 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri } } } + else if (ch == '#' && previous == '\n') + { + code << ch; + previous = ch; + inPreprocessorLine = true; + } else { - // Not whitespace and not a comment. Must be code here! - // Add any pending inline suppressions that have accumulated. - if (!suppressionIDs.empty()) + if (!inPreprocessorLine) { - if (settings != NULL) + // Not whitespace, not a comment, and not preprocessor. + // Must be code here! + + // First check for a "fall through" comment match, but only + // add a suppression if the next token is 'case' or 'default' + if (_settings->_checkCodingStyle && _settings->inconclusive && fallThroughComment) { - // Add the suppressions. - for (size_t j(0); j < suppressionIDs.size(); ++j) + std::string::size_type j = str.find_first_not_of("abcdefghijklmnopqrstuvwxyz", i); + std::string tok = str.substr(i, j - i); + if (tok == "case" || tok == "default") + suppressionIDs.push_back("switchCaseFallThrough"); + fallThroughComment = false; + } + + // Add any pending inline suppressions that have accumulated. + if (!suppressionIDs.empty()) + { + if (settings != NULL) { - const std::string errmsg(settings->nomsg.addSuppression(suppressionIDs[j], filename, lineno)); - if (!errmsg.empty()) + // Add the suppressions. + for (size_t j(0); j < suppressionIDs.size(); ++j) { - writeError(filename, lineno, _errorLogger, "cppcheckError", errmsg); + const std::string errmsg(settings->nomsg.addSuppression(suppressionIDs[j], filename, lineno)); + if (!errmsg.empty()) + { + writeError(filename, lineno, _errorLogger, "cppcheckError", errmsg); + } } } + suppressionIDs.clear(); } - suppressionIDs.clear(); } // String or char constants.. diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 769f13671..bfa17306f 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -424,9 +424,13 @@ SymbolDatabase::SymbolDatabase(const Tokenizer *tokenizer, const Settings *setti else if (Token::simpleMatch(tok, "for (") && Token::simpleMatch(tok->next()->link(), ") {")) { + // save location of initialization + const Token *tok1 = tok->tokAt(2); scope = new Scope(this, tok, scope, Scope::eFor, tok->next()->link()->next()); tok = tok->next()->link()->next(); scopeList.push_back(scope); + // check for variable declaration and add it to new scope if found + scope->checkVariable(tok1, Local); } else if (Token::simpleMatch(tok, "while (") && Token::simpleMatch(tok->next()->link(), ") {")) @@ -916,7 +920,7 @@ void SymbolDatabase::addFunction(Scope **scope, const Token **tok, const Token * if (func->hasBody) { addNewFunction(scope, tok); - if (scope) + if (*scope) { (*scope)->functionOf = scope1; added = true; @@ -1401,71 +1405,78 @@ void Scope::getVariableList() else if (Token::Match(tok, ";|{|}")) continue; - // This is the start of a statement - const Token *vartok = NULL; - const Token *typetok = NULL; - const Token *typestart = tok; - - // Is it const..? - bool isConst = false; - if (tok->str() == "const") - { - tok = tok->next(); - isConst = true; - } - - // Is it a static variable? - const bool isStatic(Token::simpleMatch(tok, "static")); - if (isStatic) - { - tok = tok->next(); - } - - // Is it a mutable variable? - const bool isMutable(Token::simpleMatch(tok, "mutable")); - if (isMutable) - { - tok = tok->next(); - } - - // Is it const..? - if (tok->str() == "const") - { - tok = tok->next(); - isConst = true; - } - - bool isClass = false; - - if (Token::Match(tok, "struct|union")) - { - tok = tok->next(); - } - - bool isArray = false; - - if (isVariableDeclaration(tok, vartok, typetok, isArray)) - { - isClass = (!typetok->isStandardType() && vartok->previous()->str() != "*"); - tok = vartok->next(); - } - - // If the vartok was set in the if-blocks above, create a entry for this variable.. - if (vartok && vartok->str() != "operator") - { - if (vartok->varId() == 0 && !vartok->isBoolean()) - check->debugMessage(vartok, "Scope::getVariableList found variable \'" + vartok->str() + "\' with varid 0."); - - const Scope *scope = NULL; - - if (typetok) - scope = check->findVariableType(this, typetok); - - addVariable(vartok, typestart, vartok->previous(), varaccess, isMutable, isStatic, isConst, isClass, scope, this, isArray); - } + tok = checkVariable(tok, varaccess); } } +const Token *Scope::checkVariable(const Token *tok, AccessControl varaccess) +{ + // This is the start of a statement + const Token *vartok = NULL; + const Token *typetok = NULL; + const Token *typestart = tok; + + // Is it const..? + bool isConst = false; + if (tok->str() == "const") + { + tok = tok->next(); + isConst = true; + } + + // Is it a static variable? + const bool isStatic(Token::simpleMatch(tok, "static")); + if (isStatic) + { + tok = tok->next(); + } + + // Is it a mutable variable? + const bool isMutable(Token::simpleMatch(tok, "mutable")); + if (isMutable) + { + tok = tok->next(); + } + + // Is it const..? + if (tok->str() == "const") + { + tok = tok->next(); + isConst = true; + } + + bool isClass = false; + + if (Token::Match(tok, "struct|union")) + { + tok = tok->next(); + } + + bool isArray = false; + + if (isVariableDeclaration(tok, vartok, typetok, isArray)) + { + isClass = (!typetok->isStandardType() && vartok->previous()->str() != "*"); + tok = vartok->next(); + } + + // If the vartok was set in the if-blocks above, create a entry for this variable.. + if (vartok && vartok->str() != "operator") + { + if (vartok->varId() == 0 && !vartok->isBoolean()) + check->debugMessage(vartok, "Scope::checkVariable found variable \'" + vartok->str() + "\' with varid 0."); + + const Scope *scope = NULL; + + if (typetok) + scope = check->findVariableType(this, typetok); + + addVariable(vartok, typestart, vartok->previous(), varaccess, isMutable, isStatic, isConst, isClass, scope, this, isArray); + } + + return tok; +} + const Token* skipScopeIdentifiers(const Token* tok) { const Token* ret = tok; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index e1cb05206..27aa08d4f 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -452,6 +452,14 @@ public: AccessControl defaultAccess() const; + /** + * @brief check if statement is variable declaration and add it if it is + * @param tok pointer to start of statement + * @param varaccess access control of statement + * @return pointer to last token + */ + const Token *checkVariable(const Token *tok, AccessControl varaccess); + private: /** * @brief helper function for getVariableList() diff --git a/lib/token.cpp b/lib/token.cpp index f874d1f8e..e40a18fa8 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -107,6 +107,9 @@ void Token::deleteThis() _isName = _next->_isName; _isNumber = _next->_isNumber; _isBoolean = _next->_isBoolean; + _isUnsigned = _next->_isUnsigned; + _isSigned = _next->_isSigned; + _isLong = _next->_isLong; _isUnused = _next->_isUnused; _varId = _next->_varId; _fileIndex = _next->_fileIndex; diff --git a/lib/token.h b/lib/token.h index 769ac4467..f23aba76f 100644 --- a/lib/token.h +++ b/lib/token.h @@ -141,14 +141,26 @@ public: { return _isName; } + void isName(bool name) + { + _isName = name; + } bool isNumber() const { return _isNumber; } + void isNumber(bool number) + { + _isNumber = number; + } bool isBoolean() const { return _isBoolean; } + void isBoolean(bool boolean) + { + _isBoolean = boolean; + } bool isUnsigned() const { return _isUnsigned; diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index cd1baa565..9514dcd12 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -243,13 +243,18 @@ void Tokenizer::insertTokens(Token *dest, const Token *src, unsigned int n) dest->fileIndex(src->fileIndex()); dest->linenr(src->linenr()); dest->varId(src->varId()); + dest->isName(src->isName()); + dest->isNumber(src->isNumber()); + dest->isBoolean(src->isBoolean()); dest->isUnsigned(src->isUnsigned()); dest->isSigned(src->isSigned()); dest->isLong(src->isLong()); + dest->isUnused(src->isUnused()); src = src->next(); --n; } } + //--------------------------------------------------------------------------- Token *Tokenizer::copyTokens(Token *dest, const Token *first, const Token *last) @@ -262,9 +267,13 @@ Token *Tokenizer::copyTokens(Token *dest, const Token *first, const Token *last) tok2 = tok2->next(); tok2->fileIndex(dest->fileIndex()); tok2->linenr(dest->linenr()); + tok2->isName(tok->isName()); + tok2->isNumber(tok->isNumber()); + tok2->isBoolean(tok->isBoolean()); tok2->isUnsigned(tok->isUnsigned()); tok2->isSigned(tok->isSigned()); tok2->isLong(tok->isLong()); + tok2->isUnused(tok->isUnused()); // Check for links and fix them up if (tok2->str() == "(" || tok2->str() == "[" || tok2->str() == "{") @@ -1142,6 +1151,13 @@ void Tokenizer::simplifyTypedef() while (Token::Match(tok->tokAt(offset), "*|&|const")) pointers.push_back(tok->tokAt(offset++)->str()); + // check for invalid input + if (!tok->tokAt(offset)) + { + syntaxError(tok); + return; + } + if (Token::Match(tok->tokAt(offset), "%type%")) { // found the type name @@ -1185,19 +1201,16 @@ void Tokenizer::simplifyTypedef() tok = deleteInvalidTypedef(typeDef); continue; } + else if (!Token::Match(tok->tokAt(offset)->link(), ") ;|,")) + { + syntaxError(tok); + return; + } function = true; - if (tok->tokAt(offset)->link()->next()) - { - argStart = tok->tokAt(offset); - argEnd = tok->tokAt(offset)->link(); - tok = argEnd->next(); - } - else - { - // internal error - continue; - } + argStart = tok->tokAt(offset); + argEnd = tok->tokAt(offset)->link(); + tok = argEnd->next(); } // unhandled typedef, skip it and continue @@ -1791,6 +1804,17 @@ void Tokenizer::simplifyTypedef() if (!inCast && !inSizeof) tok2 = tok2->next(); + // reference to array? + if (tok2->str() == "&") + { + tok2 = tok2->previous(); + tok2->insertToken("("); + tok2 = tok2->tokAt(3); + tok2->insertToken(")"); + tok2 = tok2->next(); + Token::createMutualLinks(tok2, tok2->tokAt(-3)); + } + tok2 = copyTokens(tok2, arrayStart, arrayEnd); tok2 = tok2->next(); @@ -2046,7 +2070,7 @@ bool Tokenizer::tokenize(std::istream &code, { tok->str(tok->str() + c2); tok->deleteNext(); - if (c1 == '<' && tok->next()->str() == "=") + if (c1 == '<' && Token::simpleMatch(tok->next(), "=")) { tok->str("<<="); tok->deleteNext(); @@ -2676,11 +2700,23 @@ std::list Tokenizer::simplifyTemplatesGetTemplateInstantiations() // template definition.. skip it if (Token::simpleMatch(tok, "template <")) { + unsigned int level = 0; + // Goto the end of the template definition for (; tok; tok = tok->next()) { + // skip '<' .. '>' + if (tok->str() == "<") + ++level; + else if (tok->str() == ">") + { + if (level <= 1) + break; + --level; + } + // skip inner '(' .. ')' and '{' .. '}' - if (tok->str() == "{" || tok->str() == "(") + else if (tok->str() == "{" || tok->str() == "(") { // skip inner tokens. goto ')' or '}' tok = tok->link(); @@ -2703,7 +2739,7 @@ std::list Tokenizer::simplifyTemplatesGetTemplateInstantiations() if (!tok) break; } - else if (Token::Match(tok->previous(), "[{};=] %var% <") || + else if (Token::Match(tok->previous(), "[({};=] %var% <") || Token::Match(tok->tokAt(-2), "[,:] private|protected|public %var% <")) { if (templateParameters(tok->next())) @@ -3209,7 +3245,7 @@ void Tokenizer::simplifyTemplates() //while (!done) { done = true; - for (std::list::const_iterator iter1 = templates.begin(); iter1 != templates.end(); ++iter1) + for (std::list::reverse_iterator iter1 = templates.rbegin(); iter1 != templates.rend(); ++iter1) { simplifyTemplatesInstantiate(*iter1, used, expandedtemplates); } @@ -3574,6 +3610,10 @@ void Tokenizer::setVarId() --indentlevel; } + // skip parantheses.. + else if (tok2->str() == "(") + tok2 = tok2->link(); + // Found a member variable.. else if (indentlevel == 1 && tok2->varId() > 0) varlist[tok2->str()] = tok2->varId(); @@ -5374,7 +5414,39 @@ void Tokenizer:: simplifyFunctionPointers() { for (Token *tok = _tokens; tok; tok = tok->next()) { - if (tok->previous() && !Token::Match(tok->previous(), "[{};]")) + // check for function pointer cast + if (Token::Match(tok, "( %type% *| *| ( * ) (") || + Token::Match(tok, "( %type% %type% *| *| ( * ) (") || + Token::Match(tok, "static_cast < %type% *| *| ( * ) (") || + Token::Match(tok, "static_cast < %type% %type% *| *| ( * ) (")) + { + Token *tok1 = tok; + + if (tok1->str() == "static_cast") + tok1 = tok1->next(); + + tok1 = tok1->next(); + + if (Token::Match(tok1->next(), "%type%")) + tok1 = tok1->next(); + + while (tok1->next()->str() == "*") + tok1 = tok1->next(); + + // check that the cast ends + if (!Token::Match(tok1->tokAt(4)->link(), ") )|>")) + continue; + + // ok simplify this function pointer cast to an ordinary pointer cast + tok1->deleteNext(); + tok1->next()->deleteNext(); + const Token *tok2 = tok1->tokAt(2)->link(); + Token::eraseTokens(tok1->next(), tok2 ? tok2->next() : 0); + continue; + } + + // check for start of statement + else if (tok->previous() && !Token::Match(tok->previous(), "{|}|;|(|public:|protected:|private:")) continue; if (Token::Match(tok, "%type% *| *| ( * %var% ) (")) @@ -5387,8 +5459,8 @@ void Tokenizer:: simplifyFunctionPointers() while (tok->next()->str() == "*") tok = tok->next(); - // check that the declaration ends with ; - if (!Token::simpleMatch(tok->tokAt(5)->link(), ") ;")) + // check that the declaration ends + if (!Token::Match(tok->tokAt(5)->link(), ") ;|,|)|=")) continue; // ok simplify this function pointer to an ordinary pointer @@ -7052,6 +7124,7 @@ bool Tokenizer::simplifyCalculations() // keep parentheses here: int ( * * ( * compilerHookVector ) (void) ) ( ) ; // keep parentheses here: operator new [] (size_t); // keep parentheses here: Functor()(a ... ) + // keep parentheses here: ) ( var ) ; if (Token::Match(tok->next(), "( %var% ) [;),+-*/><]]") && !tok->isName() && tok->str() != ">" && @@ -7060,7 +7133,8 @@ bool Tokenizer::simplifyCalculations() !Token::simpleMatch(tok->previous(), "* )") && !Token::simpleMatch(tok->previous(), ") )") && !Token::Match(tok->tokAt(-2), "* %var% )") && - !Token::Match(tok->tokAt(-2), "%type% ( ) ( %var%") + !Token::Match(tok->tokAt(-2), "%type% ( ) ( %var%") && + !Token::Match(tok, ") ( %var% ) ;") ) { tok->deleteNext(); diff --git a/readme.txt b/readme.txt index 0b0a0ab15..b85442449 100644 --- a/readme.txt +++ b/readme.txt @@ -23,7 +23,8 @@ Compiling There are multiple compilation choices: * qmake - cross platform build tool - * Visual Studio - Windows + * Windows: Visual Studio + * Windows: Qt Creator + mingw * gnu make * g++ @@ -36,11 +37,21 @@ Compiling Visual Studio ============= - Use the cppcheck.sln file. + Use the cppcheck.sln file. The pcre dll is needed, it can be downloaded from: + http://cppcheck.sf.net/pcre-8.10-vs.zip + + Qt Creator + mingw + ================== + The PCRE dll is needed to build the CLI. It can be downloaded here: + http://software-download.name/pcre-library-windows/ gnu make ======== - make + To build Cppcheck with rules (pcre dependency): + make + + To build Cppcheck without rules (no dependencies): + make CXXFLAGS="-O2" g++ (for experts) ================= diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index f3a74a4b6..59daa3376 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -108,6 +108,7 @@ private: TEST_CASE(array_index_32); TEST_CASE(array_index_multidim); TEST_CASE(array_index_switch_in_for); + TEST_CASE(array_index_for_in_for); // FP: #2634 TEST_CASE(array_index_calculation); TEST_CASE(array_index_negative); TEST_CASE(array_index_for_decr); @@ -1189,6 +1190,19 @@ private: TODO_ASSERT_EQUALS("[test.cpp:12]: (error) Array index out of bounds\n", "", errout.str()); } + void array_index_for_in_for() + { + check("void f() {\n" + " int a[5];\n" + " for (int i = 0; i < 10; ++i) {\n" + " for (int j = i; j < 5; ++j) {\n" + " a[i] = 0;\n" + " }\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void array_index_calculation() { // #1193 - false negative: array out of bounds in loop when there is calculation diff --git a/test/testclass.cpp b/test/testclass.cpp index d49d5abe3..afa2748a2 100644 --- a/test/testclass.cpp +++ b/test/testclass.cpp @@ -2949,6 +2949,56 @@ private: " memset(&fred, 0, sizeof(fred));\n" "}\n"); ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + " std::string s;\n" + "};\n" + "class Pebbles: public Fred {};\n" + "void f()\n" + "{\n" + " Pebbles pebbles;\n" + " memset(&pebbles, 0, sizeof(pebbles));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on class that contains a 'std::string'\n", errout.str()); + + checkNoMemset("struct Stringy {\n" + " std::string inner;\n" + "};\n" + "struct Foo {\n" + " Stringy s;\n" + "};\n" + "int main() {\n" + " Foo foo;\n" + " memset(&foo, 0, sizeof(Foo));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:9]: (error) Using 'memset' on struct that contains a 'std::string'\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + " virtual ~Fred();\n" + "};\n" + "void f()\n" + "{\n" + " Fred fred;\n" + " memset(&fred, 0, sizeof(fred));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (error) Using 'memset' on class that contains a virtual method\n", errout.str()); + + checkNoMemset("class Fred\n" + "{\n" + "};\n" + "class Wilma\n" + "{\n" + " virtual ~Wilma();\n" + "};\n" + "class Pebbles: public Fred, Wilma {};\n" + "void f()\n" + "{\n" + " Pebbles pebbles;\n" + " memset(&pebbles, 0, sizeof(pebbles));\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:12]: (error) Using 'memset' on class that contains a virtual method\n", errout.str()); } void memsetOnStruct() diff --git a/test/testmemleak.cpp b/test/testmemleak.cpp index 4fbf3832b..17cd57147 100644 --- a/test/testmemleak.cpp +++ b/test/testmemleak.cpp @@ -285,6 +285,7 @@ private: // detect leak in class member function.. TEST_CASE(class1); + TEST_CASE(class2); TEST_CASE(autoptr1); TEST_CASE(if_with_and); @@ -726,6 +727,10 @@ private: // use ; dealloc ; ASSERT_EQUALS("; alloc ; use ; if return ; dealloc ;", simplifycode("; alloc ; use ; if { return ; } dealloc ;")); + + // #2635 - false negative + ASSERT_EQUALS("; alloc ; return use ; }", + simplifycode("; alloc ; if(!var) { loop { ifv { } } alloc ; } return use; }")); } @@ -2518,6 +2523,37 @@ private: ASSERT_EQUALS("[test.cpp:7]: (error) Memory leak: p\n", errout.str()); } + void class2() + { + check("class Fred {\n" + "public:\n" + " Fred() : rootNode(0) {}\n" + "\n" + "private:\n" + " struct Node {\n" + " Node(Node* p) {\n" + " parent = p;\n" + " if (parent) {\n" + " parent->children.append(this);\n" + " }\n" + " }\n" + "\n" + " ~Node() {\n" + " qDeleteAll(children);\n" + " }\n" + "\n" + " QList children;\n" + " };\n" + "\n" + " Node rootNode;\n" + "\n" + " void f() {\n" + " Node* recordNode = new Node(&rootNode);\n" + " }\n" + "};"); + ASSERT_EQUALS("", errout.str()); + } + void autoptr1() { diff --git a/test/testother.cpp b/test/testother.cpp index 5707fc264..8366b4d68 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include "preprocessor.h" #include "tokenize.h" #include "checkother.h" #include "testsuite.h" @@ -74,6 +75,7 @@ private: TEST_CASE(sizeofCalculation); TEST_CASE(switchRedundantAssignmentTest); + TEST_CASE(switchFallThroughCase); TEST_CASE(selfAssignment); TEST_CASE(testScanf1); @@ -148,6 +150,64 @@ private: checkOther.checkComparisonOfBoolWithInt(); } + class SimpleSuppressor: public ErrorLogger + { + public: + SimpleSuppressor(Settings &settings, ErrorLogger *next) + : _settings(settings), _next(next) + { } + virtual void reportOut(const std::string &outmsg) + { + _next->reportOut(outmsg); + } + virtual void reportErr(const ErrorLogger::ErrorMessage &msg) + { + if (!msg._callStack.empty() && !_settings.nomsg.isSuppressed(msg._id, msg._callStack.begin()->getfile(), msg._callStack.begin()->line)) + _next->reportErr(msg); + } + virtual void reportStatus(unsigned int index, unsigned int max) + { + _next->reportStatus(index, max); + } + private: + Settings &_settings; + ErrorLogger *_next; + }; + + void check_preprocess_suppress(const char precode[], const char *filename = NULL) + { + // Clear the error buffer.. + errout.str(""); + + if (filename == NULL) + filename = "test.cpp"; + + Settings settings; + settings._checkCodingStyle = true; + settings.inconclusive = true; + + // Preprocess file.. + Preprocessor preprocessor(&settings, this); + std::list configurations; + std::string filedata = ""; + std::istringstream fin(precode); + preprocessor.preprocess(fin, filedata, configurations, filename, settings._includePaths); + SimpleSuppressor logger(settings, this); + const std::string code = Preprocessor::getcode(filedata, "", filename, &settings, &logger); + + // Tokenize.. + Tokenizer tokenizer(&settings, &logger); + std::istringstream istr(code); + tokenizer.tokenize(istr, filename); + tokenizer.simplifyGoto(); + + // Check.. + CheckOther checkOther(&tokenizer, &settings, &logger); + checkOther.checkSwitchCaseFallThrough(); + + logger.reportUnmatchedSuppressions(settings.nomsg.getUnmatchedLocalSuppressions(filename)); + } + void zeroDiv1() { @@ -1146,6 +1206,291 @@ private: ASSERT_EQUALS("", errout.str()); } + void switchFallThroughCase() + { + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " break;\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 0:\n" + " case 1:\n" + " break;\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " default:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " // fall through\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " /* FALLTHRU */\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " break;\n" + " // fall through\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (information) Unmatched suppression: switchCaseFallThrough\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " {\n" + " break;\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " for (;;) {\n" + " break;\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " if (b) {\n" + " break;\n" + " } else {\n" + " break;\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " if (b) {\n" + " break;\n" + " } else {\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " if (b) {\n" + " break;\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " if (b) {\n" + " } else {\n" + " break;\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:8]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " if (b) {\n" + " case 2:\n" + " } else {\n" + " break;\n" + " }\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " int x;\n" + " case 1:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " switch (b) {\n" + " case 1:\n" + " return;\n" + " default:\n" + " return;\n" + " }\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + // This fails because the switch parsing code currently doesn't understand + // that all paths after g() actually return. It's a pretty unusual case + // (no pun intended). + TODO_ASSERT_EQUALS("", + "[test.cpp:11]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + "#ifndef A\n" + " g();\n" + " // fall through\n" + "#endif\n" + " case 2:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " goto leave;\n" + " case 2:\n" + " break;\n" + " }\n" + "leave:\n" + " if (x) {\n" + " g();\n" + " return;\n" + " }\n" + "}\n"); + // This fails because Tokenizer::simplifyGoto() copies the "leave:" block + // into where the goto is, but because it contains a "return", it omits + // copying a final return after the block. + TODO_ASSERT_EQUALS("", + "[test.cpp:5]: (style) Switch falls through case without comment\n", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " switch (a) {\n" + " case 1:\n" + " g();\n" + " // fall through\n" + " case 2:\n" + " g();\n" + " // falls through\n" + " case 3:\n" + " g();\n" + " // fall-through\n" + " case 4:\n" + " g();\n" + " // drop through\n" + " case 5:\n" + " g();\n" + " // pass through\n" + " case 5:\n" + " g();\n" + " // no break\n" + " case 5:\n" + " g();\n" + " // fallthru\n" + " case 6:\n" + " g();\n" + " /* fall */\n" + " default:\n" + " break;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check_preprocess_suppress( + "void foo() {\n" + " // unrelated comment saying 'fall through'\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void selfAssignment() { check("void foo()\n" @@ -1896,16 +2241,6 @@ private: " return \"Hello\" == test.substr( 0 , 5 ) ? : 0 : 1 ;\n" "}"); ASSERT_EQUALS("", errout.str()); - - check("int f() {\n" - " return strncmp(\"test\" , \"test\" , 2) ; \n" - "}"); - ASSERT_EQUALS("[test.cpp:2]: (warning) String literal \"test\" doesn't match length argument for strncmp(2).\n", errout.str()); - - check("int f() {\n" - " return strncmp(\"test\" , \"test\" , 4) ; \n" - "}"); - ASSERT_EQUALS("", errout.str()); } diff --git a/test/testsimplifytokens.cpp b/test/testsimplifytokens.cpp index a546b3654..74adc52af 100644 --- a/test/testsimplifytokens.cpp +++ b/test/testsimplifytokens.cpp @@ -113,6 +113,7 @@ private: TEST_CASE(template20); TEST_CASE(template21); TEST_CASE(template22); + TEST_CASE(template23); TEST_CASE(template_unhandled); TEST_CASE(template_default_parameter); TEST_CASE(template_default_type); @@ -243,6 +244,8 @@ private: TEST_CASE(simplifyTypedef80); // ticket #2587 TEST_CASE(simplifyTypedef81); // ticket #2603 TEST_CASE(simplifyTypedef82); // ticket #2403 + TEST_CASE(simplifyTypedef83); // ticket #2620 + TEST_CASE(simplifyTypedef84); // ticket #2630 TEST_CASE(simplifyTypedefFunction1); TEST_CASE(simplifyTypedefFunction2); // ticket #1685 @@ -1711,7 +1714,7 @@ private: "} ;\n"; // The expected result.. - std::string expected("; void f ( ) { A a ; } ; class A { }"); + std::string expected("; void f ( ) { A a ; } ; class A { } class A { }"); ASSERT_EQUALS(expected, sizeof_(code)); } @@ -1865,18 +1868,13 @@ private: " return 0;\n" "}\n"; - const std::string wanted("; " - "; " - "int main ( ) { b<2> ( ) ; return 0 ; } " - "void b<2> ( ) { a<2> ( ) ; } " - "void a<2> ( ) { }"); + const std::string expected("; " + "int main ( ) { b<2> ( ) ; return 0 ; } " + "void b<2> ( ) { a<2> ( ) ; } " + "void a ( ) { } " + "void a<2> ( ) { }"); - const std::string current("; " - "int main ( ) { b<2> ( ) ; return 0 ; } " - "void b<2> ( ) { a < 2 > ( ) ; }" - ); - - TODO_ASSERT_EQUALS(wanted, current, sizeof_(code)); + ASSERT_EQUALS(expected, sizeof_(code)); } void template17() @@ -2024,6 +2022,22 @@ private: ASSERT_EQUALS(expected, sizeof_(code)); } + void template23() + { + const char code[] = "template void foo() { }\n" + "void bar() {\n" + " std::cout << (foo());\n" + "}"; + + const std::string expected("; " + "void bar ( ) {" + " std :: cout << ( foo ( ) ) ; " + "} " + "void foo ( ) { }"); + + ASSERT_EQUALS(expected, sizeof_(code)); + } + void template_unhandled() { @@ -3556,8 +3570,8 @@ private: const char expected[] = "; " - "void addCallback ( bool ( * callback ) ( int i ) ) { } " - "void addCallback1 ( bool ( * callback ) ( int i ) , int j ) { }"; + "void addCallback ( bool * callback ) { } " + "void addCallback1 ( bool * callback , int j ) { }"; ASSERT_EQUALS(expected, tok(code, false)); } @@ -3573,28 +3587,12 @@ private: const char expected[] = "; " - "void g ( int ( * f ) ( ) ) " + "void g ( int * f ) " "{ " - "int ( * f2 ) ( ) = ( int ( * ) ( ) ) f ; " + "int * f2 ; f2 = ( int * ) f ; " "}"; ASSERT_EQUALS(expected, tok(code, false)); - - // TODO: the definition and assignment should be split up - const char wanted[] = - "; " - "void g ( fp f ) " - "{ " - "int ( * f2 ) ( ) ; f2 = ( int ( * ) ( ) ) f ; " - "}"; - const char current[] = - "; " - "void g ( int ( * f ) ( ) ) " - "{ " - "int ( * f2 ) ( ) = ( int ( * ) ( ) ) f ; " - "}"; - - TODO_ASSERT_EQUALS(wanted, current, tok(code, false)); } { @@ -3606,9 +3604,9 @@ private: const char expected[] = "; " - "void g ( int ( * f ) ( ) ) " + "void g ( int * f ) " "{ " - "int ( * f2 ) ( ) = static_cast < int ( * ) ( ) > ( f ) ; " + "int * f2 ; f2 = static_cast < int * > ( f ) ; " "}"; ASSERT_EQUALS(expected, tok(code, false)); @@ -4961,6 +4959,33 @@ private: ASSERT_EQUALS("", errout.str()); } + void simplifyTypedef83() // ticket #2620 + { + const char code[] = "typedef char Str[10];\n" + "void f(Str &cl) { }\n"; + + // The expected result.. + const std::string expected("; " + "void f ( char ( & cl ) [ 10 ] ) { }"); + + ASSERT_EQUALS(expected, sizeof_(code)); + } + + void simplifyTypedef84() // ticket #2630 (segmentation fault) + { + const char code1[] = "typedef y x () x\n"; + checkSimplifyTypedef(code1); + ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str()); + + const char code2[] = "typedef struct template <>\n"; + checkSimplifyTypedef(code2); + ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str()); + + const char code3[] = "typedef ::<>\n"; + checkSimplifyTypedef(code3); + ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str()); + } + void simplifyTypedefFunction1() { { diff --git a/test/teststl.cpp b/test/teststl.cpp index 748c46ac0..15b68fd36 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -113,7 +113,6 @@ private: errout.str(""); Settings settings; - settings.inconclusive = true; settings._checkCodingStyle = true; // Tokenize.. @@ -1064,6 +1063,23 @@ private: void size1() { + check("struct Fred {\n" + " void foo();\n" + " std::list x;\n" + "};\n" + "void Fred::foo()\n" + "{\n" + " if (x.size() == 0) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + + check("std::list x;\n" + "void f()\n" + "{\n" + " if (x.size() == 0) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1071,6 +1087,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (0 == x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1078,6 +1101,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (0 != x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1085,6 +1115,13 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (0 < x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" @@ -1092,12 +1129,26 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" + "{\n" + " std::list x;\n" + " if (!x.size()) {}\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (performance) Possible inefficient checking for 'x' emptiness.\n", errout.str()); + check("void f()\n" "{\n" " std::list x;\n" " fun(x.size());\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("void f()\n" + "{\n" + " std::list x;\n" + " fun(!x.size());\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void redundantCondition1() diff --git a/test/testtokenize.cpp b/test/testtokenize.cpp index 04697796f..6b316e3f1 100644 --- a/test/testtokenize.cpp +++ b/test/testtokenize.cpp @@ -49,6 +49,7 @@ private: TEST_CASE(tokenize13); // bailout if the code contains "@" - that is not handled well. TEST_CASE(tokenize14); // tokenize "0X10" => 16 TEST_CASE(tokenize15); // tokenize ".123" + TEST_CASE(tokenize16); // #2612 - segfault for "<><<" // don't freak out when the syntax is wrong TEST_CASE(wrong_syntax); @@ -189,6 +190,7 @@ private: TEST_CASE(varidclass6); TEST_CASE(varidclass7); TEST_CASE(varidclass8); + TEST_CASE(varidclass9); TEST_CASE(file1); TEST_CASE(file2); @@ -516,6 +518,12 @@ private: ASSERT_EQUALS("0.125", tokenizeAndStringify(".125")); } + // #2612 - segfault for "<><<" + void tokenize16() + { + tokenizeAndStringify("<><<"); + } + void wrong_syntax() { { @@ -3356,6 +3364,32 @@ private: ASSERT_EQUALS(expected, tokenizeDebugListing(code)); } + void varidclass9() + { + const std::string code("typedef char Str[10];" + "class A {\n" + "public:\n" + " void f(Str &cl);\n" + " void g(Str cl);\n" + "}\n" + "void Fred::f(Str &cl) {\n" + " sizeof(cl);\n" + "}"); + + const std::string expected("\n\n" + "##file 0\n" + "1: ; class A {\n" + "2: public:\n" + "3: void f ( char ( & cl ) [ 10 ] ) ;\n" + "4: void g ( char cl@1 [ 10 ] ) ;\n" + "5: }\n" + "6: void Fred :: f ( char ( & cl ) [ 10 ] ) {\n" + "7: sizeof ( cl ) ;\n" + "8: }\n"); + + ASSERT_EQUALS(expected, tokenizeDebugListing(code)); + } + void file1() {