From a0906140a6c0f7336f7a803dfd2d7f51774c366b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 9 Apr 2018 06:43:48 +0200 Subject: [PATCH] Suppressions: New extensible Suppressions xml format that allow more attributes. To start with it also allows symbolName. --- .travis_suppressions | 10 +- cli/cmdlineparser.cpp | 9 + cli/threadexecutor.cpp | 20 +- gui/checkthread.cpp | 15 +- gui/checkthread.h | 5 +- gui/erroritem.cpp | 1 + gui/erroritem.h | 1 + gui/gui.pro | 21 +- gui/mainwindow.cpp | 33 +++- gui/newsuppressiondialog.cpp | 31 +++ gui/newsuppressiondialog.h | 25 +++ gui/newsuppressiondialog.ui | 115 +++++++++++ gui/projectfile.cpp | 69 ++++++- gui/projectfile.h | 14 +- gui/projectfiledialog.cpp | 40 ++-- gui/projectfiledialog.h | 10 +- gui/threadhandler.h | 5 +- lib/checkassert.cpp | 12 +- lib/checkautovariables.cpp | 9 +- lib/checkbufferoverrun.cpp | 33 +++- lib/checkclass.cpp | 95 +++++---- lib/checkfunctions.cpp | 17 +- lib/checkleakautovar.cpp | 6 +- lib/checkmemoryleak.cpp | 21 +- lib/checknullpointer.cpp | 6 +- lib/checkother.cpp | 104 ++++++---- lib/checkstl.cpp | 75 +++++--- lib/checkstring.cpp | 11 +- lib/checktype.cpp | 4 +- lib/checkuninitvar.cpp | 10 +- lib/checkunusedfunctions.cpp | 2 +- lib/checkunusedvar.cpp | 14 +- lib/cppcheck.cpp | 37 +--- lib/errorlogger.cpp | 70 +++++-- lib/errorlogger.h | 12 +- lib/preprocessor.cpp | 11 +- lib/suppressions.cpp | 361 +++++++++++++++++++---------------- lib/suppressions.h | 141 +++++++------- test/testcmdlineparser.cpp | 20 +- test/testerrorlogger.cpp | 22 +-- test/testsuppressions.cpp | 109 ++++++++--- test/testutils.h | 2 +- 42 files changed, 1062 insertions(+), 566 deletions(-) create mode 100644 gui/newsuppressiondialog.cpp create mode 100644 gui/newsuppressiondialog.h create mode 100644 gui/newsuppressiondialog.ui diff --git a/.travis_suppressions b/.travis_suppressions index f32d49a30..c7a20f912 100644 --- a/.travis_suppressions +++ b/.travis_suppressions @@ -10,10 +10,10 @@ knownConditionTrueFalse:build/* nullPointer:lib/checkother.cpp nullPointer:build/checkother.cpp -*:gui/test* +*:gui/test/* *:test/test.cxx -*:test/cfg* +*:test/cfg/* -*:externals* -*:htmlreport* -*:samples* +*:externals/*/* +*:htmlreport/* +*:samples/*/bad.c* diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 64ad809e8..194f6ff9e 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -228,6 +228,15 @@ bool CmdLineParser::ParseFromArgs(int argc, const char* const argv[]) } } + else if (std::strncmp(argv[i], "--suppress-xml=", 15) == 0) { + const char * filename = argv[i] + 15; + const std::string errmsg(_settings->nomsg.parseXmlFile(filename)); + if (!errmsg.empty()) { + PrintMessage(errmsg); + return false; + } + } + else if (std::strncmp(argv[i], "--suppress=", 11) == 0) { const std::string suppression = argv[i]+11; const std::string errmsg(_settings->nomsg.addSuppressionLine(suppression)); diff --git a/cli/threadexecutor.cpp b/cli/threadexecutor.cpp index 1b487492c..73d2f74f8 100644 --- a/cli/threadexecutor.cpp +++ b/cli/threadexecutor.cpp @@ -117,14 +117,7 @@ int ThreadExecutor::handleRead(int rpipe, unsigned int &result) ErrorLogger::ErrorMessage msg; msg.deserialize(buf); - std::string file; - unsigned int line(0); - if (!msg._callStack.empty()) { - file = msg._callStack.back().getfile(false); - line = msg._callStack.back().line; - } - - if (!_settings.nomsg.isSuppressed(msg._id, file, line)) { + if (!_settings.nomsg.isSuppressed(msg.toSuppressionsErrorMessage())) { // Alert only about unique errors std::string errmsg = msg.toString(_settings.verbose); if (std::find(_errorList.begin(), _errorList.end(), errmsg) == _errorList.end()) { @@ -309,7 +302,7 @@ unsigned int ThreadExecutor::check() "cppcheckError", false); - if (!_settings.nomsg.isSuppressed(errmsg._id, childname, 0)) + if (!_settings.nomsg.isSuppressed(errmsg.toSuppressionsErrorMessage())) _errorLogger.reportErr(errmsg); } } @@ -502,14 +495,7 @@ void ThreadExecutor::reportInfo(const ErrorLogger::ErrorMessage &msg) void ThreadExecutor::report(const ErrorLogger::ErrorMessage &msg, MessageType msgType) { - std::string file; - unsigned int line(0); - if (!msg._callStack.empty()) { - file = msg._callStack.back().getfile(false); - line = msg._callStack.back().line; - } - - if (_settings.nomsg.isSuppressed(msg._id, file, line)) + if (_settings.nomsg.isSuppressed(msg.toSuppressionsErrorMessage())) return; // Alert only about unique errors diff --git a/gui/checkthread.cpp b/gui/checkthread.cpp index dd9b1e2ed..de0f02b2b 100644 --- a/gui/checkthread.cpp +++ b/gui/checkthread.cpp @@ -419,7 +419,20 @@ void CheckThread::parseClangErrors(const QString &tool, const QString &file0, QS foreach (const ErrorItem &e, errorItems) { if (e.errorPath.isEmpty()) continue; - if (mSuppressions.contains(e.errorId)) + Suppressions::ErrorMessage errorMessage; + errorMessage.setFileName(e.errorPath.back().file.toStdString()); + errorMessage.lineNumber = e.errorPath.back().line; + errorMessage.errorId = e.errorId.toStdString(); + errorMessage.symbolNames = e.symbolNames.toStdString(); + + bool isSuppressed = false; + foreach (const Suppressions::Suppression &suppression, mSuppressions) { + if (suppression.isSuppressed(errorMessage)) { + isSuppressed = true; + break; + } + } + if (isSuppressed) continue; std::list callstack; foreach (const QErrorPathItem &path, e.errorPath) { diff --git a/gui/checkthread.h b/gui/checkthread.h index d67f01270..635be04f6 100644 --- a/gui/checkthread.h +++ b/gui/checkthread.h @@ -23,6 +23,7 @@ #include #include "cppcheck.h" #include "threadresult.h" +#include "suppressions.h" class Settings; @@ -68,7 +69,7 @@ public: mClangIncludePaths = s; } - void setSuppressions(const QStringList s) { + void setSuppressions(const QList &s) { mSuppressions = s; } @@ -151,7 +152,7 @@ private: QStringList mAddonsAndTools; QString mDataDir; QStringList mClangIncludePaths; - QStringList mSuppressions; + QList mSuppressions; QString mMisraFile; }; /// @} diff --git a/gui/erroritem.cpp b/gui/erroritem.cpp index 1caf56787..6c1a6cc33 100644 --- a/gui/erroritem.cpp +++ b/gui/erroritem.cpp @@ -46,6 +46,7 @@ ErrorItem::ErrorItem(const ErrorLogger::ErrorMessage &errmsg) , summary(QString::fromStdString(errmsg.shortMessage())) , message(QString::fromStdString(errmsg.verboseMessage())) , cwe(errmsg._cwe.id) + , symbolNames(QString::fromStdString(errmsg.symbolNames())) { for (std::list::const_iterator loc = errmsg._callStack.begin(); loc != errmsg._callStack.end(); diff --git a/gui/erroritem.h b/gui/erroritem.h index f1d59b4a7..9eeb076b6 100644 --- a/gui/erroritem.h +++ b/gui/erroritem.h @@ -89,6 +89,7 @@ public: QString message; int cwe; QList errorPath; + QString symbolNames; // Special GUI properties QString sinceDate; diff --git a/gui/gui.pro b/gui/gui.pro index 0c72ef1f6..349633f61 100644 --- a/gui/gui.pro +++ b/gui/gui.pro @@ -59,7 +59,8 @@ FORMS = about.ui \ stats.ui \ librarydialog.ui \ libraryaddfunctiondialog.ui \ - libraryeditargdialog.ui + libraryeditargdialog.ui \ + newsuppressiondialog.ui TRANSLATIONS = cppcheck_de.ts \ cppcheck_es.ts \ @@ -113,10 +114,11 @@ HEADERS += aboutdialog.h \ txtreport.h \ xmlreport.h \ xmlreportv2.h \ - librarydialog.h \ - cppchecklibrarydata.h \ - libraryaddfunctiondialog.h \ - libraryeditargdialog.h + librarydialog.h \ + cppchecklibrarydata.h \ + libraryaddfunctiondialog.h \ + libraryeditargdialog.h \ + newsuppressiondialog.h SOURCES += aboutdialog.cpp \ application.cpp \ @@ -149,10 +151,11 @@ SOURCES += aboutdialog.cpp \ txtreport.cpp \ xmlreport.cpp \ xmlreportv2.cpp \ - librarydialog.cpp \ - cppchecklibrarydata.cpp \ - libraryaddfunctiondialog.cpp \ - libraryeditargdialog.cpp + librarydialog.cpp \ + cppchecklibrarydata.cpp \ + libraryaddfunctiondialog.cpp \ + libraryeditargdialog.cpp \ + newsuppressiondialog.cpp win32 { RC_FILE = cppcheck-gui.rc diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index 022e46ce2..04e97204e 100644 --- a/gui/mainwindow.cpp +++ b/gui/mainwindow.cpp @@ -832,9 +832,9 @@ Settings MainWindow::getCppcheckSettings() tryLoadLibrary(&result.library, filename); } - const QStringList suppressions = mProjectFile->getSuppressions(); - foreach (QString suppression, suppressions) { - result.nomsg.addSuppressionLine(suppression.toStdString()); + const QList &suppressions = mProjectFile->getSuppressions(); + foreach (const Suppressions::Suppression &suppression, suppressions) { + result.nomsg.addSuppression(suppression); } // Only check the given -D configuration @@ -1729,13 +1729,26 @@ void MainWindow::tagged() void MainWindow::suppressIds(QStringList ids) { - if (mProjectFile) { - QStringList suppressions = mProjectFile->getSuppressions(); - foreach (QString s, ids) { - if (!suppressions.contains(s)) - suppressions << s; + if (!mProjectFile) + return; + ids.removeDuplicates(); + + QList suppressions = mProjectFile->getSuppressions(); + foreach (QString id, ids) { + // Remove all matching suppressions + std::string id2 = id.toStdString(); + for (int i = 0; i < suppressions.size();) { + if (suppressions[i].errorId == id2) + suppressions.removeAt(i); + else + ++i; } - mProjectFile->setSuppressions(suppressions); - mProjectFile->write(); + + Suppressions::Suppression newSuppression; + newSuppression.errorId = id2; + suppressions << newSuppression; } + + mProjectFile->setSuppressions(suppressions); + mProjectFile->write(); } diff --git a/gui/newsuppressiondialog.cpp b/gui/newsuppressiondialog.cpp new file mode 100644 index 000000000..9a7ce6333 --- /dev/null +++ b/gui/newsuppressiondialog.cpp @@ -0,0 +1,31 @@ +#include "newsuppressiondialog.h" +#include "ui_newsuppressiondialog.h" + +NewSuppressionDialog::NewSuppressionDialog(QWidget *parent) : + QDialog(parent), + mUI(new Ui::NewSuppressionDialog) +{ + mUI->setupUi(this); +} + +NewSuppressionDialog::~NewSuppressionDialog() +{ + delete mUI; +} + +void NewSuppressionDialog::setErrorIds(const QStringList &errorIds) +{ + mUI->mComboErrorId->addItems(errorIds); + mUI->mComboErrorId->setCurrentIndex(-1); + mUI->mComboErrorId->setCurrentText(""); +} + +Suppressions::Suppression NewSuppressionDialog::getSuppression() const +{ + Suppressions::Suppression ret; + ret.errorId = mUI->mComboErrorId->currentText().toStdString(); + ret.fileName = mUI->mTextFileName->text().toStdString(); + ret.lineNumber = mUI->mTextLineNumber->text().toInt(); + ret.symbolName = mUI->mTextSymbolName->text().toStdString(); + return ret; +} diff --git a/gui/newsuppressiondialog.h b/gui/newsuppressiondialog.h new file mode 100644 index 000000000..90f10f81d --- /dev/null +++ b/gui/newsuppressiondialog.h @@ -0,0 +1,25 @@ +#ifndef NEWSUPPRESSIONDIALOG_H +#define NEWSUPPRESSIONDIALOG_H + +#include +#include "suppressions.h" + +namespace Ui { + class NewSuppressionDialog; +} + +class NewSuppressionDialog : public QDialog { + Q_OBJECT + +public: + explicit NewSuppressionDialog(QWidget *parent = 0); + ~NewSuppressionDialog(); + + void setErrorIds(const QStringList &errorIds); + Suppressions::Suppression getSuppression() const; + +private: + Ui::NewSuppressionDialog *mUI; +}; + +#endif // NEWSUPPRESSIONDIALOG_H diff --git a/gui/newsuppressiondialog.ui b/gui/newsuppressiondialog.ui new file mode 100644 index 000000000..bdeb5bf2a --- /dev/null +++ b/gui/newsuppressiondialog.ui @@ -0,0 +1,115 @@ + + + NewSuppressionDialog + + + Qt::ApplicationModal + + + + 0 + 0 + 334 + 184 + + + + New suppression + + + + + + + + Error ID + + + + + + + File name + + + + + + + + + + Line number + + + + + + + + + + Symbol name + + + + + + + + + + true + + + + + + + + + Qt::Horizontal + + + QDialogButtonBox::Cancel|QDialogButtonBox::Ok + + + + + + + + + buttonBox + accepted() + NewSuppressionDialog + accept() + + + 248 + 254 + + + 157 + 274 + + + + + buttonBox + rejected() + NewSuppressionDialog + reject() + + + 316 + 260 + + + 286 + 274 + + + + + diff --git a/gui/projectfile.cpp b/gui/projectfile.cpp index 7ad9469ef..04c32f7cd 100644 --- a/gui/projectfile.cpp +++ b/gui/projectfile.cpp @@ -156,7 +156,7 @@ bool ProjectFile::read(const QString &filename) // Find suppressions list from inside project element if (insideProject && xmlReader.name() == SuppressionsElementName) - readStringList(mSuppressions, xmlReader,SuppressionElementName); + readSuppressions(xmlReader); // Addons if (insideProject && xmlReader.name() == AddonsElementName) @@ -464,6 +464,51 @@ void ProjectFile::readPlatform(QXmlStreamReader &reader) } +void ProjectFile::readSuppressions(QXmlStreamReader &reader) +{ + QXmlStreamReader::TokenType type; + do { + type = reader.readNext(); + switch (type) { + case QXmlStreamReader::StartElement: + // Read library-elements + if (reader.name().toString() == SuppressionElementName) { + Suppressions::Suppression suppression; + if (reader.attributes().hasAttribute(QString(),"fileName")) + suppression.fileName = reader.attributes().value(QString(),"fileName").toString().toStdString(); + if (reader.attributes().hasAttribute(QString(),"lineNumber")) + suppression.lineNumber = reader.attributes().value(QString(),"lineNumber").toInt(); + if (reader.attributes().hasAttribute(QString(),"symbolName")) + suppression.symbolName = reader.attributes().value(QString(),"symbolName").toString().toStdString(); + type = reader.readNext(); + if (type == QXmlStreamReader::Characters) { + suppression.errorId = reader.text().toString().toStdString(); + } + mSuppressions << suppression; + } + break; + + case QXmlStreamReader::EndElement: + if (reader.name().toString() != SuppressionElementName) + return; + break; + + // Not handled + case QXmlStreamReader::NoToken: + case QXmlStreamReader::Invalid: + case QXmlStreamReader::StartDocument: + case QXmlStreamReader::EndDocument: + case QXmlStreamReader::Characters: + case QXmlStreamReader::Comment: + case QXmlStreamReader::DTD: + case QXmlStreamReader::EntityReference: + case QXmlStreamReader::ProcessingInstruction: + break; + } + } while (true); +} + + void ProjectFile::readStringList(QStringList &stringlist, QXmlStreamReader &reader, const char elementname[]) { QXmlStreamReader::TokenType type; @@ -532,7 +577,7 @@ void ProjectFile::setPlatform(const QString &platform) mPlatform = platform; } -void ProjectFile::setSuppressions(const QStringList &suppressions) +void ProjectFile::setSuppressions(const QList &suppressions) { mSuppressions = suppressions; } @@ -630,10 +675,22 @@ bool ProjectFile::write(const QString &filename) LibrariesElementName, LibraryElementName); - writeStringList(xmlWriter, - mSuppressions, - SuppressionsElementName, - SuppressionElementName); + if (!mSuppressions.isEmpty()) { + xmlWriter.writeStartElement(SuppressionsElementName); + foreach (const Suppressions::Suppression &suppression, mSuppressions) { + xmlWriter.writeStartElement(SuppressionElementName); + if (!suppression.fileName.empty()) + xmlWriter.writeAttribute("fileName", QString::fromStdString(suppression.fileName)); + if (suppression.lineNumber > 0) + xmlWriter.writeAttribute("lineNumber", QString::number(suppression.lineNumber)); + if (!suppression.symbolName.empty()) + xmlWriter.writeAttribute("symbolName", QString::fromStdString(suppression.symbolName)); + if (!suppression.errorId.empty()) + xmlWriter.writeCharacters(QString::fromStdString(suppression.errorId)); + xmlWriter.writeEndElement(); + } + xmlWriter.writeEndElement(); + } writeStringList(xmlWriter, mAddons, diff --git a/gui/projectfile.h b/gui/projectfile.h index bb6084e4a..c31e93d24 100644 --- a/gui/projectfile.h +++ b/gui/projectfile.h @@ -24,6 +24,8 @@ #include #include +#include "suppressions.h" + /// @addtogroup GUI /// @{ @@ -118,7 +120,7 @@ public: * @brief Get list suppressions. * @return list of suppressions. */ - QStringList getSuppressions() const { + QList getSuppressions() const { return mSuppressions; } @@ -224,7 +226,7 @@ public: * @brief Set list of suppressions. * @param suppressions List of suppressions. */ - void setSuppressions(const QStringList &suppressions); + void setSuppressions(const QList &suppressions); /** * @brief Set list of addons. @@ -302,6 +304,12 @@ protected: */ void readPlatform(QXmlStreamReader &reader); + /** + * @brief Read suppressions. + * @param reader XML stream reader. + */ + void readSuppressions(QXmlStreamReader &reader); + /** * @brief Read string list * @param stringlist destination string list @@ -387,7 +395,7 @@ private: /** * @brief List of suppressions. */ - QStringList mSuppressions; + QList mSuppressions; /** * @brief List of addons. diff --git a/gui/projectfiledialog.cpp b/gui/projectfiledialog.cpp index 77f811aca..fde42bee2 100644 --- a/gui/projectfiledialog.cpp +++ b/gui/projectfiledialog.cpp @@ -27,6 +27,7 @@ #include #include #include "common.h" +#include "newsuppressiondialog.h" #include "projectfiledialog.h" #include "checkthread.h" #include "projectfile.h" @@ -484,17 +485,6 @@ QStringList ProjectFileDialog::getLibraries() const return libraries; } -QStringList ProjectFileDialog::getSuppressions() const -{ - QStringList suppressions; - const int count = mUI.mListSuppressions->count(); - for (int i = 0; i < count; i++) { - QListWidgetItem *item = mUI.mListSuppressions->item(i); - suppressions << item->text(); - } - return suppressions; -} - void ProjectFileDialog::setRootPath(const QString &root) { mUI.mEditProjectRoot->setText(QDir::toNativeSeparators(root)); @@ -553,10 +543,17 @@ void ProjectFileDialog::setLibraries(const QStringList &libraries) } } -void ProjectFileDialog::setSuppressions(const QStringList &suppressions) +void ProjectFileDialog::setSuppressions(const QList &suppressions) { + mSuppressions = suppressions; + + QStringList s; + foreach (const Suppressions::Suppression &suppression, mSuppressions) { + s << QString::fromStdString(suppression.getText()); + } + mUI.mListSuppressions->clear(); - mUI.mListSuppressions->addItems(suppressions); + mUI.mListSuppressions->addItems(s); mUI.mListSuppressions->sortItems(); } @@ -655,12 +652,10 @@ void ProjectFileDialog::addSuppression() cppcheck.getErrorMessages(); errorLogger.errorIds.sort(); - bool ok; - QString item = QInputDialog::getItem(this, tr("Add Suppression"), - tr("Select error id suppress:"), errorLogger.errorIds, 0, false, &ok); - if (ok && !item.isEmpty()) { - mUI.mListSuppressions->addItem(item); - mUI.mListSuppressions->sortItems(); + NewSuppressionDialog dlg; + dlg.setErrorIds(errorLogger.errorIds); + if (dlg.exec() == QDialog::Accepted) { + setSuppressions(mSuppressions << dlg.getSuppression()); } } @@ -668,7 +663,14 @@ void ProjectFileDialog::removeSuppression() { const int row = mUI.mListSuppressions->currentRow(); QListWidgetItem *item = mUI.mListSuppressions->takeItem(row); + const std::string s = item->text().toStdString(); delete item; + for (int i = 0; i < mSuppressions.size(); ++i) { + if (mSuppressions[i].getText() == s) { + mSuppressions.removeAt(i); + break; + } + } } void ProjectFileDialog::browseMisraFile() diff --git a/gui/projectfiledialog.h b/gui/projectfiledialog.h index e9531ecfd..0cdf76bdd 100644 --- a/gui/projectfiledialog.h +++ b/gui/projectfiledialog.h @@ -24,6 +24,8 @@ #include #include +#include "suppressions.h" + #include "ui_projectfiledialog.h" class QWidget; @@ -95,7 +97,9 @@ private: * @brief Return suppressions from the dialog control. * @return List of suppressions. */ - QStringList getSuppressions() const; + QList getSuppressions() const { + return mSuppressions; + } /** * @brief Set project root path to dialog control. @@ -142,7 +146,7 @@ private: * @brief Set suppressions to dialog control. * @param suppressions List of suppressions to set to dialog control. */ - void setSuppressions(const QStringList &suppressions); + void setSuppressions(const QList &suppressions); protected slots: @@ -277,6 +281,8 @@ private: QList mLibraryCheckboxes; QString getExistingDirectory(const QString &caption, bool trailingSlash); + + QList mSuppressions; }; /// @} diff --git a/gui/threadhandler.h b/gui/threadhandler.h index aa1219146..46bea403a 100644 --- a/gui/threadhandler.h +++ b/gui/threadhandler.h @@ -26,6 +26,7 @@ #include #include "threadresult.h" #include "importproject.h" +#include "suppressions.h" class ResultsView; class CheckThread; @@ -76,7 +77,7 @@ public: mMisraFile = misraFile; } - void setSuppressions(const QStringList &s) { + void setSuppressions(const QList &s) { mSuppressions = s; } @@ -255,7 +256,7 @@ protected: bool mAnalyseWholeProgram; QStringList mAddonsAndTools; - QStringList mSuppressions; + QList mSuppressions; QStringList mClangIncludePaths; QString mDataDir; diff --git a/lib/checkassert.cpp b/lib/checkassert.cpp index 2c609aff9..a478944f2 100644 --- a/lib/checkassert.cpp +++ b/lib/checkassert.cpp @@ -96,8 +96,10 @@ void CheckAssert::assertWithSideEffects() void CheckAssert::sideEffectInAssertError(const Token *tok, const std::string& functionName) { reportError(tok, Severity::warning, - "assertWithSideEffect", "Assert statement calls a function which may have desired side effects: '" + functionName + "'.\n" - "Non-pure function: '" + functionName + "' is called inside assert statement. " + "assertWithSideEffect", + "$symbol:" + functionName + "\n" + "Assert statement calls a function which may have desired side effects: '$symbol'.\n" + "Non-pure function: '$symbol' is called inside assert statement. " "Assert statements are removed from release builds so the code inside " "assert statement is not executed. If the code is needed also in release " "builds, this is a bug.", CWE398, false); @@ -106,8 +108,10 @@ void CheckAssert::sideEffectInAssertError(const Token *tok, const std::string& f void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& varname) { reportError(tok, Severity::warning, - "assignmentInAssert", "Assert statement modifies '" + varname + "'.\n" - "Variable '" + varname + "' is modified insert assert statement. " + "assignmentInAssert", + "$symbol:" + varname + "\n" + "Assert statement modifies '$symbol'.\n" + "Variable '$symbol' is modified insert assert statement. " "Assert statements are removed from release builds so the code inside " "assert statement is not executed. If the code is needed also in release " "builds, this is a bug.", CWE398, false); diff --git a/lib/checkautovariables.cpp b/lib/checkautovariables.cpp index 9693c3188..df74cf7b1 100644 --- a/lib/checkautovariables.cpp +++ b/lib/checkautovariables.cpp @@ -392,7 +392,7 @@ void CheckAutoVariables::errorAssignAddressOfLocalArrayToGlobalPointer(const Tok const std::string pointerName = pointer ? pointer->str() : std::string("pointer"); const std::string arrayName = array ? array->str() : std::string("array"); reportError(pointer, Severity::warning, "autoVariablesAssignGlobalPointer", - "Address of local array " + arrayName + " is assigned to global pointer " + pointerName +" and not reassigned before " + arrayName + " goes out of scope.", CWE562, false); + "$symbol:" + arrayName + "\nAddress of local array $symbol is assigned to global pointer " + pointerName +" and not reassigned before $symbol goes out of scope.", CWE562, false); } void CheckAutoVariables::errorAssignAddressOfLocalVariableToGlobalPointer(const Token *pointer, const Token *variable) @@ -400,14 +400,15 @@ void CheckAutoVariables::errorAssignAddressOfLocalVariableToGlobalPointer(const const std::string pointerName = pointer ? pointer->str() : std::string("pointer"); const std::string variableName = variable ? variable->str() : std::string("variable"); reportError(pointer, Severity::warning, "autoVariablesAssignGlobalPointer", - "Address of local variable " + variableName + " is assigned to global pointer " + pointerName +" and not reassigned before " + variableName + " goes out of scope.", CWE562, false); + "$symbol:" + variableName + "\nAddress of local variable $symbol is assigned to global pointer " + pointerName +" and not reassigned before $symbol goes out of scope.", CWE562, false); } void CheckAutoVariables::errorReturnAddressOfFunctionParameter(const Token *tok, const std::string &varname) { reportError(tok, Severity::error, "returnAddressOfFunctionParameter", - "Address of function parameter '" + varname + "' returned.\n" - "Address of the function parameter '" + varname + "' becomes invalid after the function exits because " + "$symbol:" + varname + "\n" + "Address of function parameter '$symbol' returned.\n" + "Address of the function parameter '$symbol' becomes invalid after the function exits because " "function parameters are stored on the stack which is freed when the function exits. Thus the returned " "value is invalid.", CWE562, false); } diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 7652270fb..2f4ed2fea 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -62,6 +62,7 @@ static const CWE CWE788(788U); // Access of Memory Location After End of Buffer static void makeArrayIndexOutOfBoundsError(std::ostream& oss, const CheckBufferOverrun::ArrayInfo &arrayInfo, const std::vector &index) { + oss << "$symbol:" << arrayInfo.varname() << '\n'; oss << "Array '" << arrayInfo.varname(); for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) oss << "[" << arrayInfo.num(i) << "]"; @@ -117,7 +118,8 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra return; std::ostringstream errmsg; - errmsg << ValueFlow::eitherTheConditionIsRedundant(condition) << " or the array '" << arrayInfo.varname(); + errmsg << "$symbol:" << arrayInfo.varname() << '\n'; + errmsg << ValueFlow::eitherTheConditionIsRedundant(condition) << " or the array '$symbol"; for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) errmsg << "[" << arrayInfo.num(i) << "]"; if (index.size() == 1) @@ -132,7 +134,8 @@ void CheckBufferOverrun::arrayIndexOutOfBoundsError(const Token *tok, const Arra reportError(errorPath, Severity::warning, "arrayIndexOutOfBoundsCond", errmsg.str(), CWE119, inconclusive); } else { std::ostringstream errmsg; - errmsg << "Array '" << arrayInfo.varname(); + errmsg << "$symbol:" << arrayInfo.varname() << '\n'; + errmsg << "Array '$symbol"; for (std::size_t i = 0; i < arrayInfo.num().size(); ++i) errmsg << "[" << arrayInfo.num(i) << "]"; if (index.size() == 1) @@ -161,7 +164,7 @@ static std::string bufferOverrunMessage(std::string varnames) std::string errmsg("Buffer is accessed out of bounds"); if (!varnames.empty()) - errmsg += ": " + varnames; + errmsg = "$symbol:" + varnames + '\n' + errmsg + ": " + varnames; else errmsg += "."; @@ -254,8 +257,11 @@ void CheckBufferOverrun::sizeArgumentAsCharError(const Token *tok) void CheckBufferOverrun::terminateStrncpyError(const Token *tok, const std::string &varname) { + const std::string shortMessage = "The buffer '$symbol' may not be null-terminated after the call to strncpy()."; reportError(tok, Severity::warning, "terminateStrncpy", - "The buffer '" + varname + "' may not be null-terminated after the call to strncpy().\n" + "$symbol:" + varname + '\n' + + shortMessage + '\n' + + shortMessage + ' ' + "If the source string's size fits or exceeds the given size, strncpy() does not add a " "zero at the end of the buffer. This causes bugs later in the code if the code " "assumes buffer is null-terminated.", CWE170, true); @@ -268,7 +274,9 @@ void CheckBufferOverrun::cmdLineArgsError(const Token *tok) void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const std::string &varname, const std::string &function) { - const std::string errmsg = "The buffer '" + varname + "' is not null-terminated after the call to " + function + "().\n" + const std::string errmsg = "$symbol:" + varname + '\n' + + "$symbol:" + function + '\n' + + "The buffer '" + varname + "' is not null-terminated after the call to " + function + "().\n" "The buffer '" + varname + "' is not null-terminated after the call to " + function + "(). " "This will cause bugs later in the code if the code assumes the buffer is null-terminated."; @@ -277,7 +285,10 @@ void CheckBufferOverrun::bufferNotZeroTerminatedError(const Token *tok, const st void CheckBufferOverrun::argumentSizeError(const Token *tok, const std::string &functionName, const std::string &varname) { - reportError(tok, Severity::warning, "argumentSize", "The array '" + varname + "' is too small, the function '" + functionName + "' expects a bigger one.", CWE398, false); + reportError(tok, Severity::warning, "argumentSize", + "$symbol:" + functionName + '\n' + + "$symbol:" + varname + '\n' + + "The array '" + varname + "' is too small, the function '" + functionName + "' expects a bigger one.", CWE398, false); } void CheckBufferOverrun::negativeMemoryAllocationSizeError(const Token *tok) @@ -1112,8 +1123,11 @@ void CheckBufferOverrun::negativeArraySize() void CheckBufferOverrun::negativeArraySizeError(const Token *tok) { + const std::string arrayName = tok ? tok->expressionString() : std::string(); + const std::string line1 = arrayName.empty() ? std::string() : ("$symbol:" + arrayName + '\n'); reportError(tok, Severity::error, "negativeArraySize", - "Declaration of array '" + (tok ? tok->str() : std::string()) + "' with negative size is undefined behaviour", CWE758, false); + line1 + + "Declaration of array '" + arrayName + "' with negative size is undefined behaviour", CWE758, false); } //--------------------------------------------------------------------------- @@ -1954,8 +1968,9 @@ void CheckBufferOverrun::arrayIndexThenCheck() void CheckBufferOverrun::arrayIndexThenCheckError(const Token *tok, const std::string &indexName) { reportError(tok, Severity::style, "arrayIndexThenCheck", - "Array index '" + indexName + "' is used before limits check.\n" - "Defensive programming: The variable '" + indexName + "' is used as an array index before it " + "$symbol:" + indexName + "\n" + "Array index '$symbol' is used before limits check.\n" + "Defensive programming: The variable '$symbol' is used as an array index before it " "is checked that is within limits. This can mean that the array might be accessed out of bounds. " "Reorder conditions such as '(a[i] && i < 10)' to '(i < 10 && a[i])'. That way the array will " "not be accessed if the index is out of limits.", CWE398, false); diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp index 02f1fafd3..f3e8c5165 100644 --- a/lib/checkclass.cpp +++ b/lib/checkclass.cpp @@ -375,15 +375,15 @@ void CheckClass::copyConstructorMallocError(const Token *cctor, const Token *all void CheckClass::copyConstructorShallowCopyError(const Token *tok, const std::string& varname) { reportError(tok, Severity::style, "copyCtorPointerCopying", - "Value of pointer '" + varname + "', which points to allocated memory, is copied in copy constructor instead of allocating new memory.", CWE398, false); + "$symbol:" + varname + "\nValue of pointer '$symbol', which points to allocated memory, is copied in copy constructor instead of allocating new memory.", CWE398, false); } void CheckClass::noCopyConstructorError(const Token *tok, const std::string &classname, bool isStruct) { // The constructor might be intentionally missing. Therefore this is not a "warning" reportError(tok, Severity::style, "noCopyConstructor", - std::string(isStruct ? "struct" : "class") + " '" + classname + - "' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.", CWE398, false); + "$symbol:" + classname + "\n" + + std::string(isStruct ? "struct" : "class") + " '$symbol' does not have a copy constructor which is recommended since the class contains a pointer to allocated memory.", CWE398, false); } bool CheckClass::canNotCopy(const Scope *scope) @@ -802,29 +802,28 @@ void CheckClass::noConstructorError(const Token *tok, const std::string &classna { // For performance reasons the constructor might be intentionally missing. Therefore this is not a "warning" reportError(tok, Severity::style, "noConstructor", - "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + - "' does not have a constructor.\n" - "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + - "' does not have a constructor although it has private member variables. " - "Member variables of builtin types are left uninitialized when the class is " - "instantiated. That may cause bugs or undefined behavior.", CWE398, false); + "$symbol:" + classname + "\n" + + "The " + std::string(isStruct ? "struct" : "class") + " '$symbol' does not have a constructor.\n" + "The " + std::string(isStruct ? "struct" : "class") + " '$symbol' does not have a constructor " + "although it has private member variables. Member variables of builtin types are left " + "uninitialized when the class is instantiated. That may cause bugs or undefined behavior.", CWE398, false); } void CheckClass::noExplicitConstructorError(const Token *tok, const std::string &classname, bool isStruct) { - const std::string message(std::string(isStruct ? "Struct" : "Class") + " '" + classname + "' has a constructor with 1 argument that is not explicit."); + const std::string message(std::string(isStruct ? "Struct" : "Class") + " '$symbol' has a constructor with 1 argument that is not explicit."); const std::string verbose(message + " Such constructors should in general be explicit for type safety reasons. Using the explicit keyword in the constructor means some mistakes when using the class can be avoided."); - reportError(tok, Severity::style, "noExplicitConstructor", message + "\n" + verbose, CWE398, false); + reportError(tok, Severity::style, "noExplicitConstructor", "$symbol:" + classname + '\n' + message + '\n' + verbose, CWE398, false); } void CheckClass::uninitVarError(const Token *tok, bool isprivate, const std::string &classname, const std::string &varname, bool inconclusive) { - reportError(tok, Severity::warning, isprivate ? "uninitMemberVarPrivate" : "uninitMemberVar", "Member variable '" + classname + "::" + varname + "' is not initialized in the constructor.", CWE398, inconclusive); + reportError(tok, Severity::warning, isprivate ? "uninitMemberVarPrivate" : "uninitMemberVar", "$symbol:" + classname + "::" + varname + "\nMember variable '$symbol' is not initialized in the constructor.", CWE398, inconclusive); } void CheckClass::operatorEqVarError(const Token *tok, const std::string &classname, const std::string &varname, bool inconclusive) { - reportError(tok, Severity::warning, "operatorEqVarError", "Member variable '" + classname + "::" + varname + "' is not assigned a value in '" + classname + "::operator='.", CWE398, inconclusive); + reportError(tok, Severity::warning, "operatorEqVarError", "$symbol:" + classname + "::" + varname + "\nMember variable '$symbol' is not assigned a value in '" + classname + "::operator='.", CWE398, inconclusive); } //--------------------------------------------------------------------------- @@ -887,10 +886,10 @@ void CheckClass::initializationListUsage() void CheckClass::suggestInitializationList(const Token* tok, const std::string& varname) { - reportError(tok, Severity::performance, "useInitializationList", "Variable '" + varname + "' is assigned in constructor body. Consider performing initialization in initialization list.\n" + reportError(tok, Severity::performance, "useInitializationList", "$symbol:" + varname + "\nVariable '$symbol' is assigned in constructor body. Consider performing initialization in initialization list.\n" "When an object of a class is created, the constructors of all member variables are called consecutively " "in the order the variables are declared, even if you don't explicitly write them to the initialization list. You " - "could avoid assigning '" + varname + "' a value by passing the value to the constructor in the initialization list.", CWE398, false); + "could avoid assigning '$symbol' a value by passing the value to the constructor in the initialization list.", CWE398, false); } //--------------------------------------------------------------------------- @@ -1000,7 +999,7 @@ void CheckClass::privateFunctions() void CheckClass::unusedPrivateFunctionError(const Token *tok, const std::string &classname, const std::string &funcname) { - reportError(tok, Severity::style, "unusedPrivateFunction", "Unused private function: '" + classname + "::" + funcname + "'", CWE398, false); + reportError(tok, Severity::style, "unusedPrivateFunction", "$symbol:" + classname + "::" + funcname + "\nUnused private function: '$symbol'", CWE398, false); } //--------------------------------------------------------------------------- @@ -1164,8 +1163,9 @@ void CheckClass::mallocOnClassWarning(const Token* tok, const std::string &memfu toks.push_back(tok); toks.push_back(classTok); reportError(toks, Severity::warning, "mallocOnClassWarning", - "Memory for class instance allocated with " + memfunc + "(), but class provides constructors.\n" - "Memory for class instance allocated with " + memfunc + "(), but class provides constructors. This is unsafe, " + "$symbol:" + memfunc +"\n" + "Memory for class instance allocated with $symbol(), but class provides constructors.\n" + "Memory for class instance allocated with $symbol(), but class provides constructors. This is unsafe, " "since no constructor is called and class members remain uninitialized. Consider using 'new' instead.", CWE762, false); } @@ -1175,6 +1175,8 @@ void CheckClass::mallocOnClassError(const Token* tok, const std::string &memfunc toks.push_back(tok); toks.push_back(classTok); reportError(toks, Severity::error, "mallocOnClassError", + "$symbol:" + memfunc +"\n" + "$symbol:" + classname +"\n" "Memory for class instance allocated with " + memfunc + "(), but class contains a " + classname + ".\n" "Memory for class instance allocated with " + memfunc + "(), but class a " + classname + ". This is unsafe, " "since no constructor is called and class members remain uninitialized. Consider using 'new' instead.", CWE665, false); @@ -1183,6 +1185,8 @@ void CheckClass::mallocOnClassError(const Token* tok, const std::string &memfunc void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const std::string &classname, const std::string &type) { reportError(tok, Severity::error, "memsetClass", + "$symbol:" + memfunc +"\n" + "$symbol:" + classname +"\n" "Using '" + memfunc + "' on " + type + " that contains a " + classname + ".\n" "Using '" + memfunc + "' on " + type + " that contains a " + classname + " is unsafe, because constructor, destructor " "and copy operator calls are omitted. These are necessary for this non-POD type to ensure that a valid object " @@ -1191,7 +1195,9 @@ void CheckClass::memsetError(const Token *tok, const std::string &memfunc, const void CheckClass::memsetErrorReference(const Token *tok, const std::string &memfunc, const std::string &type) { - reportError(tok, Severity::error, "memsetClassReference", "Using '" + memfunc + "' on " + type + " that contains a reference.", CWE665, false); + reportError(tok, Severity::error, "memsetClassReference", + "$symbol:" + memfunc +"\n" + "Using '" + memfunc + "' on " + type + " that contains a reference.", CWE665, false); } void CheckClass::memsetErrorFloat(const Token *tok, const std::string &type) @@ -1252,8 +1258,10 @@ void CheckClass::operatorEq() void CheckClass::operatorEqReturnError(const Token *tok, const std::string &className) { - reportError(tok, Severity::style, "operatorEq", "'" + className + "::operator=' should return '" + className + " &'.\n" - "The "+className+"::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '"+className+" &"+className+"::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers." + reportError(tok, Severity::style, "operatorEq", + "$symbol:" + className +"\n" + "'$symbol::operator=' should return '$symbol &'.\n" + "The $symbol::operator= does not conform to standard C/C++ behaviour. To conform to standard C/C++ behaviour, return a reference to self (such as: '$symbol &$symbol::operator=(..) { .. return *this; }'. For safety reasons it might be better to not fix this message. If you think that safety is always more important than conformance then please ignore/suppress this message. For more details about this topic, see the book \"Effective C++\" by Scott Meyers." , CWE398, false); } @@ -1653,9 +1661,12 @@ void CheckClass::virtualDestructorError(const Token *tok, const std::string &Bas { if (inconclusive) { if (_settings->isEnabled(Settings::WARNING)) - reportError(tok, Severity::warning, "virtualDestructor", "Class '" + Base + "' which has virtual members does not have a virtual destructor.", CWE404, true); + reportError(tok, Severity::warning, "virtualDestructor", "$symbol:" + Base + "\nClass '$symbol' which has virtual members does not have a virtual destructor.", CWE404, true); } else { - reportError(tok, Severity::error, "virtualDestructor", "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor.\n" + reportError(tok, Severity::error, "virtualDestructor", + "$symbol:" + Base +"\n" + "$symbol:" + Derived +"\n" + "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor.\n" "Class '" + Base + "' which is inherited by class '" + Derived + "' does not have a virtual destructor. " "If you destroy instances of the derived class by deleting a pointer that points to the base class, only " "the destructor of the base class is executed. Thus, dynamic memory that is managed by the derived class " @@ -2045,16 +2056,18 @@ void CheckClass::checkConstError2(const Token *tok1, const Token *tok2, const st toks.push_back(tok2); if (!suggestStatic) reportError(toks, Severity::style, "functionConst", - "Technically the member function '" + classname + "::" + funcname + "' can be const.\n" - "The member function '" + classname + "::" + funcname + "' can be made a const " + "$symbol:" + classname + "::" + funcname +"\n" + "Technically the member function '$symbol' can be const.\n" + "The member function '$symbol' can be made a const " "function. Making this function 'const' should not cause compiler errors. " "Even though the function can be made const function technically it may not make " "sense conceptually. Think about your design and the task of the function first - is " "it a function that must not change object internal state?", CWE398, true); else reportError(toks, Severity::performance, "functionStatic", - "Technically the member function '" + classname + "::" + funcname + "' can be static.\n" - "The member function '" + classname + "::" + funcname + "' can be made a static " + "$symbol:" + classname + "::" + funcname +"\n" + "Technically the member function '$symbol' can be static.\n" + "The member function '$symbol' can be made a static " "function. Making a function static can bring a performance benefit since no 'this' instance is " "passed to the function. This change should not cause compiler errors but it does not " "necessarily make sense conceptually. Think about your design and the task of the function first - " @@ -2137,10 +2150,9 @@ void CheckClass::initializerListError(const Token *tok1, const Token *tok2, cons toks.push_back(tok1); toks.push_back(tok2); reportError(toks, Severity::style, "initializerList", - "Member variable '" + classname + "::" + - varname + "' is in the wrong place in the initializer list.\n" - "Member variable '" + classname + "::" + - varname + "' is in the wrong place in the initializer list. " + "$symbol:" + classname + "::" + varname +"\n" + "Member variable '$symbol' is in the wrong place in the initializer list.\n" + "Member variable '$symbol' is in the wrong place in the initializer list. " "Members are initialized in the order they are declared, not in the " "order they are in the initializer list. Keeping the initializer list " "in the same order that the members were declared prevents order dependent " @@ -2174,7 +2186,7 @@ void CheckClass::checkSelfInitialization() void CheckClass::selfInitializationError(const Token* tok, const std::string& varname) { - reportError(tok, Severity::error, "selfInitialization", "Member variable '" + varname + "' is initialized by itself.", CWE665, false); + reportError(tok, Severity::error, "selfInitialization", "$symbol:" + varname + "\nMember variable '$symbol' is initialized by itself.", CWE665, false); } @@ -2336,8 +2348,10 @@ void CheckClass::pureVirtualFunctionCallInConstructorError( if (!errorPath.empty()) errorPath.back().second = purefuncname + " is a pure virtual method without body"; - reportError(tokStack, Severity::warning, "pureVirtualCall", "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ".\n" - "Call of pure virtual function '" + purefuncname + "' in " + scopeFunctionTypeName + ". The call will fail during runtime.", CWE(0U), false); + reportError(tokStack, Severity::warning, "pureVirtualCall", + "$symbol:" + purefuncname +"\n" + "Call of pure virtual function '$symbol' in " + scopeFunctionTypeName + ".\n" + "Call of pure virtual function '$symbol' in " + scopeFunctionTypeName + ". The call will fail during runtime.", CWE(0U), false); } @@ -2388,10 +2402,12 @@ void CheckClass::duplInheritedMembersError(const Token *tok1, const Token* tok2, toks.push_back(tok1); toks.push_back(tok2); + const std::string symbols = "$symbol:" + derivedname + "\n$symbol:" + variablename + "\n$symbol:" + basename; + const std::string message = "The " + std::string(derivedIsStruct ? "struct" : "class") + " '" + derivedname + "' defines member variable with name '" + variablename + "' also defined in its parent " + std::string(baseIsStruct ? "struct" : "class") + " '" + basename + "'."; - reportError(toks, Severity::warning, "duplInheritedMember", message, CWE398, false); + reportError(toks, Severity::warning, "duplInheritedMember", symbols + '\n' + message, CWE398, false); } @@ -2456,11 +2472,11 @@ void CheckClass::checkCopyCtorAndEqOperator() void CheckClass::copyCtorAndEqOperatorError(const Token *tok, const std::string &classname, bool isStruct, bool hasCopyCtor) { - const std::string message = "The " + std::string(isStruct ? "struct" : "class") + " '" + classname + - "' has '" + getFunctionTypeName(hasCopyCtor ? Function::eCopyConstructor : Function::eOperatorEqual) + + const std::string message = "$symbol:" + classname + "\n" + "The " + std::string(isStruct ? "struct" : "class") + " '$symbol' has '" + + getFunctionTypeName(hasCopyCtor ? Function::eCopyConstructor : Function::eOperatorEqual) + "' but lack of '" + getFunctionTypeName(hasCopyCtor ? Function::eOperatorEqual : Function::eCopyConstructor) + "'."; - reportError(tok, Severity::warning, "copyCtorAndEqOperator", message); } @@ -2506,6 +2522,7 @@ void CheckClass::checkUnsafeClassDivZero(bool test) void CheckClass::unsafeClassDivZeroError(const Token *tok, const std::string &className, const std::string &methodName, const std::string &varName) { + const std::string symbols = "$symbol:" + className + "\n$symbol:" + methodName + "\n$symbol:" + varName + '\n'; const std::string s = className + "::" + methodName + "()"; - reportError(tok, Severity::style, "unsafeClassDivZero", "Public interface of " + className + " is not safe. When calling " + s + ", if parameter " + varName + " is 0 that leads to division by zero."); + reportError(tok, Severity::style, "unsafeClassDivZero", symbols + "Public interface of " + className + " is not safe. When calling " + s + ", if parameter " + varName + " is 0 that leads to division by zero."); } diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index 0af7b09a1..c5fce517b 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -66,12 +66,14 @@ void CheckFunctions::checkProhibitedFunctions() if (_tokenizer->isC()) { if (_settings->standards.c > Standards::C89) reportError(tok, Severity::warning, "allocaCalled", + "$symbol:alloca\n" "Obsolete function 'alloca' called. In C99 and later it is recommended to use a variable length array instead.\n" "The obsolete function 'alloca' is called. In C99 and later it is recommended to use a variable length array or " "a dynamically allocated array instead. The function 'alloca' is dangerous for many reasons " "(http://stackoverflow.com/questions/1018853/why-is-alloca-not-considered-good-practice and http://linux.die.net/man/3/alloca)."); } else reportError(tok, Severity::warning, "allocaCalled", + "$symbol:alloca\n" "Obsolete function 'alloca' called.\n" "The obsolete function 'alloca' is called. In C++11 and later it is recommended to use std::array<> or " "a dynamically allocated array instead. The function 'alloca' is dangerous for many reasons " @@ -133,12 +135,12 @@ void CheckFunctions::invalidFunctionUsage() void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const ValueFlow::Value *invalidValue, const std::string &validstr) { std::ostringstream errmsg; + errmsg << "$symbol:" << functionName << '\n'; if (invalidValue && invalidValue->condition) errmsg << ValueFlow::eitherTheConditionIsRedundant(invalidValue->condition) - << " or " << functionName << "() argument nr " << argnr - << " can have invalid value."; + << " or $symbol() argument nr " << argnr << " can have invalid value."; else - errmsg << "Invalid " << functionName << "() argument nr " << argnr << '.'; + errmsg << "Invalid $symbol() argument nr " << argnr << '.'; if (invalidValue) errmsg << " The value is " << invalidValue->intvalue << " but the valid values are '" << validstr << "'."; else @@ -162,7 +164,8 @@ void CheckFunctions::invalidFunctionArgError(const Token *tok, const std::string void CheckFunctions::invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr) { std::ostringstream errmsg; - errmsg << "Invalid " << functionName << "() argument nr " << argnr << ". A non-boolean value is required."; + errmsg << "$symbol:" << functionName << '\n'; + errmsg << "Invalid $symbol() argument nr " << argnr << ". A non-boolean value is required."; reportError(tok, Severity::error, "invalidFunctionArgBool", errmsg.str(), CWE628, false); } @@ -205,7 +208,7 @@ void CheckFunctions::checkIgnoredReturnValue() void CheckFunctions::ignoredReturnValueError(const Token* tok, const std::string& function) { reportError(tok, Severity::warning, "ignoredReturnValue", - "Return value of function " + function + "() is not used.", CWE252, false); + "$symbol:" + function + "\nReturn value of function $symbol() is not used.", CWE252, false); } @@ -289,9 +292,9 @@ void CheckFunctions::mathfunctionCallWarning(const Token *tok, const unsigned in { if (tok) { if (numParam == 1) - reportError(tok, Severity::warning, "wrongmathcall", "Passing value " + tok->strAt(2) + " to " + tok->str() + "() leads to implementation-defined result.", CWE758, false); + reportError(tok, Severity::warning, "wrongmathcall", "$symbol:" + tok->str() + "\nPassing value " + tok->strAt(2) + " to $symbol() leads to implementation-defined result.", CWE758, false); else if (numParam == 2) - reportError(tok, Severity::warning, "wrongmathcall", "Passing values " + tok->strAt(2) + " and " + tok->strAt(4) + " to " + tok->str() + "() leads to implementation-defined result.", CWE758, false); + reportError(tok, Severity::warning, "wrongmathcall", "$symbol:" + tok->str() + "\nPassing values " + tok->strAt(2) + " and " + tok->strAt(4) + " to $symbol() leads to implementation-defined result.", CWE758, false); } else reportError(tok, Severity::warning, "wrongmathcall", "Passing value '#' to #() leads to implementation-defined result.", CWE758, false); } diff --git a/lib/checkleakautovar.cpp b/lib/checkleakautovar.cpp index c93a14992..d14da7b03 100644 --- a/lib/checkleakautovar.cpp +++ b/lib/checkleakautovar.cpp @@ -117,7 +117,7 @@ void CheckLeakAutoVar::deallocUseError(const Token *tok, const std::string &varn void CheckLeakAutoVar::deallocReturnError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::error, "deallocret", "Returning/dereferencing '" + varname + "' after it is deallocated / released", CWE672, false); + reportError(tok, Severity::error, "deallocret", "$symbol:" + varname + "\nReturning/dereferencing '$symbol' after it is deallocated / released", CWE672, false); } void CheckLeakAutoVar::configurationInfo(const Token* tok, const std::string &functionName) @@ -133,9 +133,9 @@ void CheckLeakAutoVar::configurationInfo(const Token* tok, const std::string &fu void CheckLeakAutoVar::doubleFreeError(const Token *tok, const std::string &varname, int type) { if (_settings->library.isresource(type)) - reportError(tok, Severity::error, "doubleFree", "Resource handle '" + varname + "' freed twice.", CWE415, false); + reportError(tok, Severity::error, "doubleFree", "$symbol:" + varname + "\nResource handle '$symbol' freed twice.", CWE415, false); else - reportError(tok, Severity::error, "doubleFree", "Memory pointed to by '" + varname + "' is freed twice.", CWE415, false); + reportError(tok, Severity::error, "doubleFree", "$symbol:" + varname + "\nMemory pointed to by '$symbol' is freed twice.", CWE415, false); } diff --git a/lib/checkmemoryleak.cpp b/lib/checkmemoryleak.cpp index 8076b25d7..69e1d049d 100644 --- a/lib/checkmemoryleak.cpp +++ b/lib/checkmemoryleak.cpp @@ -309,30 +309,30 @@ void CheckMemoryLeak::reportErr(const std::list &callstack, Sever void CheckMemoryLeak::memleakError(const Token *tok, const std::string &varname) const { - reportErr(tok, Severity::error, "memleak", "Memory leak: " + varname, CWE(401U)); + reportErr(tok, Severity::error, "memleak", "$symbol:" + varname + "\nMemory leak: $symbol", CWE(401U)); } void CheckMemoryLeak::memleakUponReallocFailureError(const Token *tok, const std::string &varname) const { - reportErr(tok, Severity::error, "memleakOnRealloc", "Common realloc mistake: \'" + varname + "\' nulled but not freed upon failure", CWE(401U)); + reportErr(tok, Severity::error, "memleakOnRealloc", "$symbol:" + varname + "\nCommon realloc mistake: \'$symbol\' nulled but not freed upon failure", CWE(401U)); } void CheckMemoryLeak::resourceLeakError(const Token *tok, const std::string &varname) const { std::string errmsg("Resource leak"); if (!varname.empty()) - errmsg += ": " + varname; + errmsg = "$symbol:" + varname + '\n' + errmsg + ": $symbol"; reportErr(tok, Severity::error, "resourceLeak", errmsg, CWE(775U)); } void CheckMemoryLeak::deallocDeallocError(const Token *tok, const std::string &varname) const { - reportErr(tok, Severity::error, "deallocDealloc", "Deallocating a deallocated pointer: " + varname, CWE(415U)); + reportErr(tok, Severity::error, "deallocDealloc", "$symbol:" + varname + "\nDeallocating a deallocated pointer: $symbol", CWE(415U)); } void CheckMemoryLeak::deallocuseError(const Token *tok, const std::string &varname) const { - reportErr(tok, Severity::error, "deallocuse", "Dereferencing '" + varname + "' after it is deallocated / released", CWE(416U)); + reportErr(tok, Severity::error, "deallocuse", "$symbol:" + varname + "\nDereferencing '$symbol' after it is deallocated / released", CWE(416U)); } void CheckMemoryLeak::mismatchSizeError(const Token *tok, const std::string &sz) const @@ -342,7 +342,7 @@ void CheckMemoryLeak::mismatchSizeError(const Token *tok, const std::string &sz) void CheckMemoryLeak::mismatchAllocDealloc(const std::list &callstack, const std::string &varname) const { - reportErr(callstack, Severity::error, "mismatchAllocDealloc", "Mismatching allocation and deallocation: " + varname, CWE(762U)); + reportErr(callstack, Severity::error, "mismatchAllocDealloc", "$symbol:" + varname + "\nMismatching allocation and deallocation: $symbol", CWE(762U)); } CheckMemoryLeak::AllocType CheckMemoryLeak::functionReturnType(const Function* func, std::list *callstack) const @@ -2386,6 +2386,8 @@ void CheckMemoryLeakInClass::unsafeClassError(const Token *tok, const std::strin return; reportError(tok, Severity::style, "unsafeClassCanLeak", + "$symbol:" + classname + "\n" + "$symbol:" + varname + "\n" "Class '" + classname + "' is unsafe, '" + varname + "' can leak by wrong usage.\n" "The class '" + classname + "' is unsafe, wrong usage can cause memory/resource leaks for '" + varname + "'. This can for instance be fixed by adding proper cleanup in the destructor.", CWE398, false); } @@ -2425,7 +2427,7 @@ void CheckMemoryLeakInClass::checkPublicFunctions(const Scope *scope, const Toke void CheckMemoryLeakInClass::publicAllocationError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::warning, "publicAllocationError", "Possible leak in public function. The pointer '" + varname + "' is not deallocated before it is allocated.", CWE398, false); + reportError(tok, Severity::warning, "publicAllocationError", "$symbol:" + varname + "\nPossible leak in public function. The pointer '$symbol' is not deallocated before it is allocated.", CWE398, false); } @@ -2774,14 +2776,15 @@ void CheckMemoryLeakNoVar::functionCallLeak(const Token *loc, const std::string void CheckMemoryLeakNoVar::returnValueNotUsedError(const Token *tok, const std::string &alloc) { - reportError(tok, Severity::error, "leakReturnValNotUsed", "Return value of allocation function '" + alloc + "' is not stored.", CWE771, false); + reportError(tok, Severity::error, "leakReturnValNotUsed", "$symbol:" + alloc + "\nReturn value of allocation function '$symbol' is not stored.", CWE771, false); } void CheckMemoryLeakNoVar::unsafeArgAllocError(const Token *tok, const std::string &funcName, const std::string &ptrType, const std::string& objType) { const std::string factoryFunc = ptrType == "shared_ptr" ? "make_shared" : "make_unique"; reportError(tok, Severity::warning, "leakUnsafeArgAlloc", - "Unsafe allocation. If " + funcName + "() throws, memory could be leaked. Use " + factoryFunc + "<" + objType + ">() instead.", + "$symbol:" + funcName + "\n" + "Unsafe allocation. If $symbol() throws, memory could be leaked. Use " + factoryFunc + "<" + objType + ">() instead.", CWE401, true); // Inconclusive because funcName may never throw } diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp index 244f0ff65..440b82074 100644 --- a/lib/checknullpointer.cpp +++ b/lib/checknullpointer.cpp @@ -480,8 +480,8 @@ void CheckNullPointer::nullConstantDereference() void CheckNullPointer::nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value *value, bool inconclusive) { - const std::string errmsgcond(ValueFlow::eitherTheConditionIsRedundant(value ? value->condition : nullptr) + " or there is possible null pointer dereference: " + varname + "."); - const std::string errmsgdefarg("Possible null pointer dereference if the default parameter value is used: " + varname); + const std::string errmsgcond("$symbol:" + varname + '\n' + ValueFlow::eitherTheConditionIsRedundant(value ? value->condition : nullptr) + " or there is possible null pointer dereference: $symbol."); + const std::string errmsgdefarg("$symbol:" + varname + "\nPossible null pointer dereference if the default parameter value is used: $symbol"); if (!tok) { reportError(tok, Severity::error, "nullPointer", "Null pointer dereference", CWE476, false); @@ -508,7 +508,7 @@ void CheckNullPointer::nullPointerError(const Token *tok, const std::string &var std::string errmsg; errmsg = std::string(value->isKnown() ? "Null" : "Possible null") + " pointer dereference"; if (!varname.empty()) - errmsg += ": " + varname; + errmsg = "$symbol:" + varname + '\n' + errmsg + ": $symbol"; reportError(errorPath, value->isKnown() ? Severity::error : Severity::warning, diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 3318bcd38..fa7113b4b 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -134,10 +134,11 @@ void CheckOther::checkCastIntToCharAndBackError(const Token *tok, const std::str tok, Severity::warning, "checkCastIntToCharAndBack", - "Storing "+ strFunctionName +"() return value in char variable and then comparing with EOF.\n" - "When saving "+ strFunctionName +"() return value in char variable there is loss of precision. " - " When "+ strFunctionName +"() returns EOF this value is truncated. Comparing the char " - "variable with EOF can have unexpected results. For instance a loop \"while (EOF != (c = "+ strFunctionName +"());\" " + "$symbol:" + strFunctionName + "\n" + "Storing $symbol() return value in char variable and then comparing with EOF.\n" + "When saving $symbol() return value in char variable there is loss of precision. " + " When $symnol() returns EOF this value is truncated. Comparing the char " + "variable with EOF can have unexpected results. For instance a loop \"while (EOF != (c = $symbol());\" " "loops forever on some compilers/platforms and on other compilers/platforms it will stop " "when the file contains a matching character.", CWE197, false ); @@ -411,9 +412,11 @@ void CheckOther::checkPipeParameterSize() void CheckOther::checkPipeParameterSizeError(const Token *tok, const std::string &strVarName, const std::string &strDim) { reportError(tok, Severity::error, - "wrongPipeParameterSize", "Buffer '" + strVarName + "' must have size of 2 integers if used as parameter of pipe().\n" + "wrongPipeParameterSize", + "$symbol:" + strVarName + "\n" + "Buffer '$symbol' must have size of 2 integers if used as parameter of pipe().\n" "The pipe()/pipe2() system command takes an argument, which is an array of exactly two integers.\n" - "The variable '" + strVarName + "' is an array of size " + strDim + ", which does not match.", CWE686, false); + "The variable '$symbol' is an array of size " + strDim + ", which does not match.", CWE686, false); } //--------------------------------------------------------------------------- @@ -711,14 +714,16 @@ void CheckOther::redundantCopyError(const Token *tok1, const Token* tok2, const { const std::list callstack = { tok1, tok2 }; reportError(callstack, Severity::performance, "redundantCopy", - "Buffer '" + var + "' is being written before its old content has been used.", CWE563, false); + "$symbol:" + var + "\n" + "Buffer '$symbol' is being written before its old content has been used.", CWE563, false); } void CheckOther::redundantCopyInSwitchError(const Token *tok1, const Token* tok2, const std::string &var) { const std::list callstack = { tok1, tok2 }; reportError(callstack, Severity::warning, "redundantCopyInSwitch", - "Buffer '" + var + "' is being written before its old content has been used. 'break;' missing?", CWE563, false); + "$symbol:" + var + "\n" + "Buffer '$symbol' is being written before its old content has been used. 'break;' missing?", CWE563, false); } void CheckOther::redundantAssignmentError(const Token *tok1, const Token* tok2, const std::string& var, bool inconclusive) @@ -726,18 +731,21 @@ void CheckOther::redundantAssignmentError(const Token *tok1, const Token* tok2, const std::list callstack = { tok1, tok2 }; if (inconclusive) reportError(callstack, Severity::style, "redundantAssignment", - "Variable '" + var + "' is reassigned a value before the old one has been used if variable is no semaphore variable.\n" - "Variable '" + var + "' is reassigned a value before the old one has been used. Make sure that this variable is not used like a semaphore in a threading environment before simplifying this code.", CWE563, true); + "$symbol:" + var + "\n" + "Variable '$symbol' is reassigned a value before the old one has been used if variable is no semaphore variable.\n" + "Variable '$symbol' is reassigned a value before the old one has been used. Make sure that this variable is not used like a semaphore in a threading environment before simplifying this code.", CWE563, true); else reportError(callstack, Severity::style, "redundantAssignment", - "Variable '" + var + "' is reassigned a value before the old one has been used.", CWE563, false); + "$symbol:" + var + "\n" + "Variable '$symbol' is reassigned a value before the old one has been used.", CWE563, false); } void CheckOther::redundantAssignmentInSwitchError(const Token *tok1, const Token* tok2, const std::string &var) { const std::list callstack = { tok1, tok2 }; reportError(callstack, Severity::warning, "redundantAssignInSwitch", - "Variable '" + var + "' is reassigned a value before the old one has been used. 'break;' missing?", CWE563, false); + "$symbol:" + var + "\n" + "Variable '$symbol' is reassigned a value before the old one has been used. 'break;' missing?", CWE563, false); } @@ -876,7 +884,9 @@ void CheckOther::checkRedundantAssignmentInSwitch() void CheckOther::redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname) { reportError(tok, Severity::warning, - "redundantBitwiseOperationInSwitch", "Redundant bitwise operation on '" + varname + "' in 'switch' statement. 'break;' missing?"); + "redundantBitwiseOperationInSwitch", + "$symbol:" + varname + "\n" + "Redundant bitwise operation on '$symbol' in 'switch' statement. 'break;' missing?"); } @@ -1258,8 +1268,9 @@ void CheckOther::variableScopeError(const Token *tok, const std::string &varname reportError(tok, Severity::style, "variableScope", - "The scope of the variable '" + varname + "' can be reduced.\n" - "The scope of the variable '" + varname + "' can be reduced. Warning: Be careful " + "$symbol:" + varname + "\n" + "The scope of the variable '$symbol' can be reduced.\n" + "The scope of the variable '$symbol' can be reduced. Warning: Be careful " "when fixing this message, especially when there are inner loops. Here is an " "example where cppcheck will write that the scope for 'i' can be reduced:\n" "void f(int x)\n" @@ -1462,8 +1473,9 @@ void CheckOther::checkPassByReference() void CheckOther::passedByValueError(const Token *tok, const std::string &parname, bool inconclusive) { reportError(tok, Severity::performance, "passedByValue", - "Function parameter '" + parname + "' should be passed by reference.\n" - "Parameter '" + parname + "' is passed by value. It could be passed " + "$symbol:" + parname + "\n" + "Function parameter '$symbol' should be passed by reference.\n" + "Parameter '$symbol' is passed by value. It could be passed " "as a (const) reference which is usually faster and recommended in C++.", CWE398, inconclusive); } @@ -1730,7 +1742,9 @@ void CheckOther::checkMisusedScopedObject() void CheckOther::misusedScopeObjectError(const Token *tok, const std::string& varname) { reportError(tok, Severity::style, - "unusedScopedObject", "Instance of '" + varname + "' object is destroyed immediately.", CWE563, false); + "unusedScopedObject", + "$symbol:" + varname + "\n" + "Instance of '$symbol' object is destroyed immediately.", CWE563, false); } //----------------------------------------------------------------------------- @@ -2044,7 +2058,9 @@ void CheckOther::duplicateValueTernaryError(const Token *tok) void CheckOther::selfAssignmentError(const Token *tok, const std::string &varname) { reportError(tok, Severity::warning, - "selfAssignment", "Redundant assignment of '" + varname + "' to itself.", CWE398, false); + "selfAssignment", + "$symbol:" + varname + "\n" + "Redundant assignment of '$symbol' to itself.", CWE398, false); } //----------------------------------------------------------------------------- @@ -2092,8 +2108,9 @@ void CheckOther::checkComparisonFunctionIsAlwaysTrueOrFalseError(const Token* to const struct CWE cweResult = result ? CWE571 : CWE570; reportError(tok, Severity::warning, "comparisonFunctionIsAlwaysTrueOrFalse", - "Comparison of two identical variables with " + functionName + "(" + varName + "," + varName + ") always evaluates to " + strResult + ".\n" - "The function " + functionName + " is designed to compare two variables. Calling this function with one variable (" + varName + ") " + "$symbol:" + functionName + "\n" + "Comparison of two identical variables with $symbol(" + varName + "," + varName + ") always evaluates to " + strResult + ".\n" + "The function $symbol is designed to compare two variables. Calling this function with one variable (" + varName + ") " "for both parameters leads to a statement which is always " + strResult + ".", cweResult, false); } @@ -2152,15 +2169,17 @@ void CheckOther::unsignedLessThanZeroError(const Token *tok, const std::string & { if (inconclusive) { reportError(tok, Severity::style, "unsignedLessThanZero", - "Checking if unsigned variable '" + varname + "' is less than zero. This might be a false warning.\n" - "Checking if unsigned variable '" + varname + "' is less than zero. An unsigned " + "$symbol:" + varname + "\n" + "Checking if unsigned variable '$symbol' is less than zero. This might be a false warning.\n" + "Checking if unsigned variable '$symbol' is less than zero. An unsigned " "variable will never be negative so it is either pointless or an error to check if it is. " "It's not known if the used constant is a template parameter or not and therefore " "this message might be a false warning.", CWE570, true); } else { reportError(tok, Severity::style, "unsignedLessThanZero", - "Checking if unsigned variable '" + varname + "' is less than zero.\n" - "The unsigned variable '" + varname + "' will never be negative so it " + "$symbol:" + varname + "\n" + "Checking if unsigned variable '$symbol' is less than zero.\n" + "The unsigned variable '$symbol' will never be negative so it " "is either pointless or an error to check if it is.", CWE570, false); } } @@ -2175,13 +2194,15 @@ void CheckOther::unsignedPositiveError(const Token *tok, const std::string &varn { if (inconclusive) { reportError(tok, Severity::style, "unsignedPositive", - "Unsigned variable '" + varname + "' can't be negative so it is unnecessary to test it.\n" - "The unsigned variable '" + varname + "' can't be negative so it is unnecessary to test it. " + "$symbol:" + varname + "\n" + "Unsigned variable '$symbol' can't be negative so it is unnecessary to test it.\n" + "The unsigned variable '$symbol' can't be negative so it is unnecessary to test it. " "It's not known if the used constant is a " "template parameter or not and therefore this message might be a false warning", CWE570, true); } else { reportError(tok, Severity::style, "unsignedPositive", - "Unsigned variable '" + varname + "' can't be negative so it is unnecessary to test it.", CWE570, false); + "$symbol:" + varname + "\n" + "Unsigned variable '$symbol' can't be negative so it is unnecessary to test it.", CWE570, false); } } @@ -2252,9 +2273,10 @@ void CheckOther::checkRedundantCopy() void CheckOther::redundantCopyError(const Token *tok,const std::string& varname) { reportError(tok, Severity::performance, "redundantCopyLocalConst", - "Use const reference for '" + varname + "' to avoid unnecessary data copying.\n" - "The const variable '"+varname+"' is assigned a copy of the data. You can avoid " - "the unnecessary data copying by converting '" + varname + "' to const reference.", + "$symbol:" + varname + "\n" + "Use const reference for '$symbol' to avoid unnecessary data copying.\n" + "The const variable '$symbol' is assigned a copy of the data. You can avoid " + "the unnecessary data copying by converting '$symbol' to const reference.", CWE398, true); // since #5618 that check became inconclusive } @@ -2359,10 +2381,14 @@ void CheckOther::incompleteArrayFillError(const Token* tok, const std::string& b { if (boolean) reportError(tok, Severity::portability, "incompleteArrayFill", + "$symbol:" + buffer + "\n" + "$symbol:" + function + "\n" "Array '" + buffer + "' might be filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n" "The array '" + buffer + "' is filled incompletely. The function '" + function + "()' needs the size given in bytes, but the type 'bool' is larger than 1 on some platforms. Did you forget to multiply the size with 'sizeof(*" + buffer + ")'?", CWE131, true); else reportError(tok, Severity::warning, "incompleteArrayFill", + "$symbol:" + buffer + "\n" + "$symbol:" + function + "\n" "Array '" + buffer + "' is filled incompletely. Did you forget to multiply the size given to '" + function + "()' with 'sizeof(*" + buffer + ")'?\n" "The array '" + buffer + "' is filled incompletely. The function '" + function + "()' needs the size given in bytes, but an element of the given array is larger than one byte. Did you forget to multiply the size with 'sizeof(*" + buffer + ")'?", CWE131, true); } @@ -2490,7 +2516,8 @@ void CheckOther::checkRedundantPointerOp() void CheckOther::redundantPointerOpError(const Token* tok, const std::string &varname, bool inconclusive) { reportError(tok, Severity::style, "redundantPointerOp", - "Redundant pointer operation on '" + varname + "' - it's already a pointer.", CWE398, inconclusive); + "$symbol:" + varname + "\n" + "Redundant pointer operation on '$symbol' - it's already a pointer.", CWE398, inconclusive); } void CheckOther::checkInterlockedDecrement() @@ -2564,11 +2591,13 @@ void CheckOther::unusedLabelError(const Token* tok, bool inSwitch) if (inSwitch) { if (!tok || _settings->isEnabled(Settings::WARNING)) reportError(tok, Severity::warning, "unusedLabelSwitch", - "Label '" + (tok ? tok->str() : emptyString) + "' is not used. Should this be a 'case' of the enclosing switch()?", CWE398, false); + "$symbol:" + (tok ? tok->str() : emptyString) + "\n" + "Label '$symbol' is not used. Should this be a 'case' of the enclosing switch()?", CWE398, false); } else { if (!tok || _settings->isEnabled(Settings::STYLE)) reportError(tok, Severity::style, "unusedLabel", - "Label '" + (tok ? tok->str() : emptyString) + "' is not used.", CWE398, false); + "$symbol:" + (tok ? tok->str() : emptyString) + "\n" + "Label '$symbol' is not used.", CWE398, false); } } @@ -2740,7 +2769,7 @@ void CheckOther::accessMovedError(const Token *tok, const std::string &varname, default: return; } - const std::string errmsg("Access of " + kindString + " variable '" + varname + "'."); + const std::string errmsg("$symbol:" + varname + "\nAccess of " + kindString + " variable '$symbol'."); const ErrorPath errorPath = getErrorPath(tok, value, errmsg); reportError(errorPath, Severity::warning, errorId, errmsg, CWE672, inconclusive); } @@ -2835,7 +2864,8 @@ void CheckOther::funcArgNamesDifferent(const std::string & functionName, size_t tokens.push_back(declaration); tokens.push_back(definition); reportError(tokens, Severity::style, "funcArgNamesDifferent", - "Function '" + functionName + "' argument " + MathLib::toString(index + 1) + " names different: declaration '" + + "$symbol:" + functionName + "\n" + "Function '$symbol' argument " + MathLib::toString(index + 1) + " names different: declaration '" + (declaration ? declaration->str() : std::string("A")) + "' definition '" + (definition ? definition->str() : std::string("B")) + "'.", CWE628, true); } @@ -2848,7 +2878,7 @@ void CheckOther::funcArgOrderDifferent(const std::string & functionName, std::list tokens; tokens.push_back(declarations.size() ? declarations[0] ? declarations[0] : declaration : nullptr); tokens.push_back(definitions.size() ? definitions[0] ? definitions[0] : definition : nullptr); - std::string msg = "Function '" + functionName + "' argument order different: declaration '"; + std::string msg = "$symbol:" + functionName + "\nFunction '$symbol' argument order different: declaration '"; for (std::size_t i = 0; i < declarations.size(); ++i) { if (i != 0) msg += ", "; diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 2a776e380..ca87a53ed 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -52,12 +52,15 @@ static const struct CWE CWE834(834U); // Excessive Iteration // Error message for bad iterator usage.. void CheckStl::invalidIteratorError(const Token *tok, const std::string &iteratorName) { - reportError(tok, Severity::error, "invalidIterator1", "Invalid iterator: " + iteratorName, CWE664, false); + reportError(tok, Severity::error, "invalidIterator1", "$symbol:"+iteratorName+"\nInvalid iterator: $symbol", CWE664, false); } void CheckStl::iteratorsError(const Token *tok, const std::string &container1, const std::string &container2) { - reportError(tok, Severity::error, "iterators", "Same iterator is used with different containers '" + container1 + "' and '" + container2 + "'.", CWE664, false); + reportError(tok, Severity::error, "iterators", + "$symbol:" + container1 + "\n" + "$symbol:" + container2 + "\n" + "Same iterator is used with different containers '" + container1 + "' and '" + container2 + "'.", CWE664, false); } // Error message used when dereferencing an iterator that has been erased.. @@ -68,13 +71,15 @@ void CheckStl::dereferenceErasedError(const Token *erased, const Token* deref, c callstack.push_back(deref); callstack.push_back(erased); reportError(callstack, Severity::error, "eraseDereference", - "Iterator '" + itername + "' used after element has been erased.\n" - "The iterator '" + itername + "' is invalid after the element it pointed to has been erased. " + "$symbol:" + itername + "\n" + "Iterator '$symbol' used after element has been erased.\n" + "The iterator '$symbol' is invalid after the element it pointed to has been erased. " "Dereferencing or comparing it with another iterator is invalid operation.", CWE664, inconclusive); } else { reportError(deref, Severity::error, "eraseDereference", - "Invalid iterator '" + itername + "' used.\n" - "The iterator '" + itername + "' is invalid before being assigned. " + "$symbol:" + itername + "\n" + "Invalid iterator '$symbol' used.\n" + "The iterator '$symbol' is invalid before being assigned. " "Dereferencing or comparing it with another iterator is invalid operation.", CWE664, inconclusive); } } @@ -461,9 +466,9 @@ void CheckStl::stlOutOfBounds() void CheckStl::stlOutOfBoundsError(const Token *tok, const std::string &num, const std::string &var, bool at) { if (at) - reportError(tok, Severity::error, "stlOutOfBounds", "When " + num + "==" + var + ".size(), " + var + ".at(" + num + ") is out of bounds.", CWE788, false); + reportError(tok, Severity::error, "stlOutOfBounds", "$symbol:" + var + "\nWhen " + num + "==$symbol.size(), $symbol.at(" + num + ") is out of bounds.", CWE788, false); else - reportError(tok, Severity::error, "stlOutOfBounds", "When " + num + "==" + var + ".size(), " + var + "[" + num + "] is out of bounds.", CWE788, false); + reportError(tok, Severity::error, "stlOutOfBounds", "$symbol:" + var + "\nWhen " + num + "==$symbol.size(), $symbol[" + num + "] is out of bounds.", CWE788, false); } void CheckStl::negativeIndex() @@ -703,14 +708,20 @@ void CheckStl::pushback() // Error message for bad iterator usage.. void CheckStl::invalidIteratorError(const Token *tok, const std::string &func, const std::string &iterator_name) { - reportError(tok, Severity::error, "invalidIterator2", "After " + func + "(), the iterator '" + iterator_name + "' may be invalid.", CWE664, false); + reportError(tok, Severity::error, "invalidIterator2", + "$symbol:" + func + "\n" + "$symbol:" + iterator_name + "\n" + "After " + func + "(), the iterator '" + iterator_name + "' may be invalid.", CWE664, false); } // Error message for bad iterator usage.. void CheckStl::invalidPointerError(const Token *tok, const std::string &func, const std::string &pointer_name) { - reportError(tok, Severity::error, "invalidPointer", "Invalid pointer '" + pointer_name + "' after " + func + "().", CWE664, false); + reportError(tok, Severity::error, "invalidPointer", + "$symbol:" + func + "\n" + "$symbol:" + pointer_name + "\n" + "Invalid pointer '" + pointer_name + "' after " + func + "().", CWE664, false); } @@ -909,10 +920,11 @@ void CheckStl::sizeError(const Token *tok) { const std::string varname = tok ? tok->str() : std::string("list"); reportError(tok, Severity::performance, "stlSize", - "Possible inefficient checking for '" + varname + "' emptiness.\n" - "Checking for '" + varname + "' emptiness might be inefficient. " - "Using " + varname + ".empty() instead of " + varname + ".size() can be faster. " + - varname + ".size() can take linear time but " + varname + ".empty() is " + "$symbol:" + varname + "\n" + "Possible inefficient checking for '$symbol' emptiness.\n" + "Checking for '$symbol' emptiness might be inefficient. " + "Using $symbol.empty() instead of $symbol.size() can be faster. " + "$symbol.size() can take linear time but $symbol.empty() is " "guaranteed to take constant time.", CWE398, false); } @@ -1373,9 +1385,9 @@ void CheckStl::autoPointerArrayError(const Token *tok) void CheckStl::autoPointerMallocError(const Token *tok, const std::string& allocFunction) { - const std::string summary = "Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with function '" + allocFunction + "'."; - const std::string verbose = summary + " This means that you should only use 'auto_ptr' for pointers obtained with operator 'new'. This excludes use C library allocation functions (for example '" + allocFunction + "'), which must be deallocated by the appropriate C library function."; - reportError(tok, Severity::error, "useAutoPointerMalloc", summary + "\n" + verbose, CWE762, false); + const std::string summary = "Object pointed by an 'auto_ptr' is destroyed using operator 'delete'. You should not use 'auto_ptr' for pointers obtained with function '$symbol'."; + const std::string verbose = summary + " This means that you should only use 'auto_ptr' for pointers obtained with operator 'new'. This excludes use C library allocation functions (for example '$symbol'), which must be deallocated by the appropriate C library function."; + reportError(tok, Severity::error, "useAutoPointerMalloc", "$symbol:" + allocFunction + '\n' + summary + '\n' + verbose, CWE762, false); } namespace { @@ -1435,6 +1447,8 @@ void CheckStl::uselessCalls() void CheckStl::uselessCallsReturnValueError(const Token *tok, const std::string &varname, const std::string &function) { std::ostringstream errmsg; + errmsg << "$symbol:" << varname << '\n'; + errmsg << "$symbol:" << function << '\n'; errmsg << "It is inefficient to call '" << varname << "." << function << "(" << varname << ")' as it always returns 0.\n" << "'std::string::" << function << "()' returns zero when given itself as parameter " << "(" << varname << "." << function << "(" << varname << ")). As it is currently the " @@ -1445,12 +1459,12 @@ void CheckStl::uselessCallsReturnValueError(const Token *tok, const std::string void CheckStl::uselessCallsSwapError(const Token *tok, const std::string &varname) { - std::ostringstream errmsg; - errmsg << "It is inefficient to swap a object with itself by calling '" << varname << ".swap(" << varname << ")'\n" - << "The 'swap()' function has no logical effect when given itself as parameter " - << "(" << varname << ".swap(" << varname << ")). As it is currently the " - << "code is inefficient. Is the object or the parameter wrong here?"; - reportError(tok, Severity::performance, "uselessCallsSwap", errmsg.str(), CWE628, false); + reportError(tok, Severity::performance, "uselessCallsSwap", + "$symbol:" + varname + "\n" + "It is inefficient to swap a object with itself by calling '$symbol.swap($symbol)'\n" + "The 'swap()' function has no logical effect when given itself as parameter " + "($symbol.swap($symbol)). As it is currently the " + "code is inefficient. Is the object or the parameter wrong here?", CWE628, false); } void CheckStl::uselessCallsSubstrError(const Token *tok, bool empty) @@ -1468,8 +1482,10 @@ void CheckStl::uselessCallsEmptyError(const Token *tok) void CheckStl::uselessCallsRemoveError(const Token *tok, const std::string& function) { - reportError(tok, Severity::warning, "uselessCallsRemove", "Return value of std::" + function + "() ignored. Elements remain in container.\n" - "The return value of std::" + function + "() is ignored. This function returns an iterator to the end of the range containing those elements that should be kept. " + reportError(tok, Severity::warning, "uselessCallsRemove", + "$symbol:" + function + "\n" + "Return value of std::$symbol() ignored. Elements remain in container.\n" + "The return value of std::$symbol() is ignored. This function returns an iterator to the end of the range containing those elements that should be kept. " "Elements past new end remain valid but with unspecified values. Use the erase method of the container to delete them.", CWE762, false); } @@ -1540,8 +1556,10 @@ void CheckStl::checkDereferenceInvalidIterator() void CheckStl::dereferenceInvalidIteratorError(const Token* deref, const std::string &iterName) { reportError(deref, Severity::warning, - "derefInvalidIterator", "Possible dereference of an invalid iterator: " + iterName + "\n" + - "Make sure to check that the iterator is valid before dereferencing it - not after.", CWE825, false); + "derefInvalidIterator", + "$symbol:" + iterName + "\n" + "Possible dereference of an invalid iterator: $symbol\n" + "Possible dereference of an invalid iterator: $symbol. Make sure to check that the iterator is valid before dereferencing it - not after.", CWE825, false); } @@ -1668,5 +1686,6 @@ void CheckStl::readingEmptyStlContainer() void CheckStl::readingEmptyStlContainerError(const Token *tok) { - reportError(tok, Severity::style, "reademptycontainer", "Reading from empty STL container '" + (tok ? tok->str() : std::string("var")) + "'", CWE398, true); + const std::string varname = tok ? tok->str() : std::string("var"); + reportError(tok, Severity::style, "reademptycontainer", "$symbol:" + varname +"\nReading from empty STL container '$symbol'", CWE398, true); } diff --git a/lib/checkstring.cpp b/lib/checkstring.cpp index 3b0135e05..6049a4fd2 100644 --- a/lib/checkstring.cpp +++ b/lib/checkstring.cpp @@ -228,13 +228,13 @@ void CheckString::checkSuspiciousStringCompare() void CheckString::suspiciousStringCompareError(const Token* tok, const std::string& var) { reportError(tok, Severity::warning, "literalWithCharPtrCompare", - "String literal compared with variable '" + var + "'. Did you intend to use strcmp() instead?", CWE595, false); + "$symbol:" + var + "\nString literal compared with variable '$symbol'. Did you intend to use strcmp() instead?", CWE595, false); } void CheckString::suspiciousStringCompareError_char(const Token* tok, const std::string& var) { reportError(tok, Severity::warning, "charLiteralWithCharPtrCompare", - "Char literal compared with pointer '" + var + "'. Did you intend to dereference it?", CWE595, false); + "$symbol:" + var + "\nChar literal compared with pointer '$symbol'. Did you intend to dereference it?", CWE595, false); } @@ -324,7 +324,7 @@ void CheckString::checkIncorrectStringCompare() void CheckString::incorrectStringCompareError(const Token *tok, const std::string& func, const std::string &string) { - reportError(tok, Severity::warning, "incorrectStringCompare", "String literal " + string + " doesn't match length argument for " + func + "().", CWE570, false); + reportError(tok, Severity::warning, "incorrectStringCompare", "$symbol:" + func + "\nString literal " + string + " doesn't match length argument for $symbol().", CWE570, false); } void CheckString::incorrectStringBooleanError(const Token *tok, const std::string& string) @@ -453,8 +453,9 @@ void CheckString::sprintfOverlappingData() void CheckString::sprintfOverlappingDataError(const Token *tok, const std::string &varname) { reportError(tok, Severity::error, "sprintfOverlappingData", - "Undefined behavior: Variable '" + varname + "' is used as parameter and destination in s[n]printf().\n" - "The variable '" + varname + "' is used both as a parameter and as destination in " + "$symbol:" + varname + "\n" + "Undefined behavior: Variable '$symbol' is used as parameter and destination in s[n]printf().\n" + "The variable '$symbol' is used both as a parameter and as destination in " "s[n]printf(). The origin and destination buffers overlap. Quote from glibc (C-library) " "documentation (http://www.gnu.org/software/libc/manual/html_mono/libc.html#Formatted-Output-Functions): " "\"If copying takes place between objects that overlap as a result of a call " diff --git a/lib/checktype.cpp b/lib/checktype.cpp index a640f7d24..808392a69 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -249,8 +249,8 @@ void CheckType::signConversionError(const Token *tok, const bool constvalue) Severity::warning, "signConversion", (constvalue) ? - "Suspicious code: sign conversion of " + varname + " in calculation because '" + varname + "' has a negative value" : - "Suspicious code: sign conversion of " + varname + " in calculation, even though " + varname + " can have a negative value", CWE195, false); + "$symbol:" + varname + "\nSuspicious code: sign conversion of $symbol in calculation because '$symbol' has a negative value" : + "$symbol:" + varname + "\nSuspicious code: sign conversion of $symbol in calculation, even though $symbol can have a negative value", CWE195, false); } diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp index fc25e6de6..ced6b26c7 100644 --- a/lib/checkuninitvar.cpp +++ b/lib/checkuninitvar.cpp @@ -1201,17 +1201,17 @@ bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, All void CheckUninitVar::uninitstringError(const Token *tok, const std::string &varname, bool strncpy_) { - reportError(tok, Severity::error, "uninitstring", "Dangerous usage of '" + varname + "'" + (strncpy_ ? " (strncpy doesn't always null-terminate it)." : " (not null-terminated)."), CWE676, false); + reportError(tok, Severity::error, "uninitstring", "$symbol:" + varname + "\nDangerous usage of '$symbol'" + (strncpy_ ? " (strncpy doesn't always null-terminate it)." : " (not null-terminated)."), CWE676, false); } void CheckUninitVar::uninitdataError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::error, "uninitdata", "Memory is allocated but not initialized: " + varname, CWE908, false); + reportError(tok, Severity::error, "uninitdata", "$symbol:" + varname + "\nMemory is allocated but not initialized: $symbol", CWE908, false); } void CheckUninitVar::uninitvarError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::error, "uninitvar", "Uninitialized variable: " + varname, CWE908, false); + reportError(tok, Severity::error, "uninitvar", "$symbol:" + varname + "\nUninitialized variable: $symbol", CWE908, false); } void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string &membername) @@ -1219,7 +1219,7 @@ void CheckUninitVar::uninitStructMemberError(const Token *tok, const std::string reportError(tok, Severity::error, "uninitStructMember", - "Uninitialized struct member: " + membername, CWE908, false); + "$symbol:" + membername + "\nUninitialized struct member: $symbol", CWE908, false); } void CheckUninitVar::valueFlowUninit() @@ -1277,5 +1277,5 @@ void CheckUninitVar::deadPointerError(const Token *pointer, const Token *alias) reportError(pointer, Severity::error, "deadpointer", - "Dead pointer usage. Pointer '" + strpointer + "' is dead if it has been assigned '" + stralias + "' at line " + MathLib::toString(alias ? alias->linenr() : 0U) + ".", CWE825, false); + "$symbol:" + strpointer + "\nDead pointer usage. Pointer '$symbol' is dead if it has been assigned '" + stralias + "' at line " + MathLib::toString(alias ? alias->linenr() : 0U) + ".", CWE825, false); } diff --git a/lib/checkunusedfunctions.cpp b/lib/checkunusedfunctions.cpp index 2f15abbd3..f1c20aed0 100644 --- a/lib/checkunusedfunctions.cpp +++ b/lib/checkunusedfunctions.cpp @@ -325,7 +325,7 @@ void CheckUnusedFunctions::unusedFunctionError(ErrorLogger * const errorLogger, locationList.push_back(fileLoc); } - const ErrorLogger::ErrorMessage errmsg(locationList, emptyString, Severity::style, "The function '" + funcname + "' is never used.", "unusedFunction", CWE561, false); + const ErrorLogger::ErrorMessage errmsg(locationList, emptyString, Severity::style, "$symbol:" + funcname + "\nThe function '$symbol' is never used.", "unusedFunction", CWE561, false); if (errorLogger) errorLogger->reportErr(errmsg); else diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 7c1180ad1..7b440f25b 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -1277,25 +1277,25 @@ void CheckUnusedVar::checkFunctionVariableUsage() void CheckUnusedVar::unusedVariableError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::style, "unusedVariable", "Unused variable: " + varname, CWE563, false); + reportError(tok, Severity::style, "unusedVariable", "$symbol:" + varname + "\nUnused variable: $symbol", CWE563, false); } void CheckUnusedVar::allocatedButUnusedVariableError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::style, "unusedAllocatedMemory", "Variable '" + varname + "' is allocated memory that is never used.", CWE563, false); + reportError(tok, Severity::style, "unusedAllocatedMemory", "$symbol:" + varname + "\nVariable '$symbol' is allocated memory that is never used.", CWE563, false); } void CheckUnusedVar::unreadVariableError(const Token *tok, const std::string &varname, bool modified) { if (modified) - reportError(tok, Severity::style, "unreadVariable", "Variable '" + varname + "' is modified but its new value is never used.", CWE563, false); + reportError(tok, Severity::style, "unreadVariable", "$symbol:" + varname + "\nVariable '$symbol' is modified but its new value is never used.", CWE563, false); else - reportError(tok, Severity::style, "unreadVariable", "Variable '" + varname + "' is assigned a value that is never used.", CWE563, false); + reportError(tok, Severity::style, "unreadVariable", "$symbol:" + varname + "\nVariable '$symbol' is assigned a value that is never used.", CWE563, false); } void CheckUnusedVar::unassignedVariableError(const Token *tok, const std::string &varname) { - reportError(tok, Severity::style, "unassignedVariable", "Variable '" + varname + "' is not assigned a value.", CWE665, false); + reportError(tok, Severity::style, "unassignedVariable", "$symbol:" + varname + "\nVariable '$symbol' is not assigned a value.", CWE665, false); } //--------------------------------------------------------------------------- @@ -1390,8 +1390,8 @@ void CheckUnusedVar::checkStructMemberUsage() void CheckUnusedVar::unusedStructMemberError(const Token *tok, const std::string &structname, const std::string &varname, bool isUnion) { - const char* prefix = isUnion ? "union member '" : "struct member '"; - reportError(tok, Severity::style, "unusedStructMember", std::string(prefix) + structname + "::" + varname + "' is never used.", CWE563, false); + const std::string prefix = isUnion ? "union member " : "struct member "; + reportError(tok, Severity::style, "unusedStructMember", "$symbol:" + structname + "::" + varname + '\n' + prefix + "'$symbol' is never used.", CWE563, false); } bool CheckUnusedVar::isRecordTypeWithoutSideEffects(const Type* type) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index faca8500d..8aef730f8 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -728,22 +728,12 @@ void CppCheck::reportErr(const ErrorLogger::ErrorMessage &msg) if (std::find(_errorList.begin(), _errorList.end(), errmsg) != _errorList.end()) return; - std::string file; - unsigned int line(0); - if (!msg._callStack.empty()) { - file = msg._callStack.back().getfile(false); - line = msg._callStack.back().line; - } + const Suppressions::ErrorMessage errorMessage = msg.toSuppressionsErrorMessage(); - if (_useGlobalSuppressions) { - if (_settings.nomsg.isSuppressed(msg._id, file, line)) - return; - } else { - if (_settings.nomsg.isSuppressedLocal(msg._id, file, line)) - return; - } + if (_settings.nomsg.isSuppressed(errorMessage)) + return; - if (!_settings.nofail.isSuppressed(msg._id, file, line) && !_settings.nomsg.isSuppressed(msg._id, file, line)) + if (!_settings.nofail.isSuppressed(errorMessage)) exitcode = 1; _errorList.push_back(errmsg); @@ -767,22 +757,9 @@ void CppCheck::reportProgress(const std::string &filename, const char stage[], c void CppCheck::reportInfo(const ErrorLogger::ErrorMessage &msg) { - // Suppressing info message? - std::string file; - unsigned int line(0); - if (!msg._callStack.empty()) { - file = msg._callStack.back().getfile(false); - line = msg._callStack.back().line; - } - if (_useGlobalSuppressions) { - if (_settings.nomsg.isSuppressed(msg._id, file, line)) - return; - } else { - if (_settings.nomsg.isSuppressedLocal(msg._id, file, line)) - return; - } - - _errorLogger.reportInfo(msg); + const Suppressions::ErrorMessage &errorMessage = msg.toSuppressionsErrorMessage(); + if (!_settings.nomsg.isSuppressed(errorMessage)) + _errorLogger.reportInfo(msg); } void CppCheck::reportStatus(unsigned int /*fileindex*/, unsigned int /*filecount*/, std::size_t /*sizedone*/, std::size_t /*sizetotal*/) diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index ca7ec0fc6..a65bddd6d 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -175,6 +175,16 @@ ErrorLogger::ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errms } } +static std::string replaceStr(std::string s, const std::string &from, const std::string &to) +{ + std::string::size_type pos = 0; + while (std::string::npos != (pos = s.find(from,pos))) { + s = s.substr(0, pos) + to + s.substr(pos + from.size()); + pos += to.size(); + } + return s; +} + void ErrorLogger::ErrorMessage::setmsg(const std::string &msg) { // If a message ends to a '\n' and contains only a one '\n' @@ -188,15 +198,33 @@ void ErrorLogger::ErrorMessage::setmsg(const std::string &msg) // If there is no newline then both the summary and verbose messages // are the given message const std::string::size_type pos = msg.find('\n'); + const std::string symbolName = _symbolNames.empty() ? std::string() : _symbolNames.substr(0, _symbolNames.find('\n')); if (pos == std::string::npos) { - _shortMessage = msg; - _verboseMessage = msg; + _shortMessage = replaceStr(msg, "$symbol", symbolName); + _verboseMessage = replaceStr(msg, "$symbol", symbolName); + } else if (msg.compare(0,8,"$symbol:") == 0) { + _symbolNames += msg.substr(8, pos-7); + setmsg(msg.substr(pos + 1)); } else { - _shortMessage = msg.substr(0, pos); - _verboseMessage = msg.substr(pos + 1); + _shortMessage = replaceStr(msg.substr(0, pos), "$symbol", symbolName); + _verboseMessage = replaceStr(msg.substr(pos + 1), "$symbol", symbolName); } } +Suppressions::ErrorMessage ErrorLogger::ErrorMessage::toSuppressionsErrorMessage() const +{ + Suppressions::ErrorMessage ret; + ret.errorId = _id; + if (!_callStack.empty()) { + ret.setFileName(_callStack.back().getfile(false)); + ret.lineNumber = _callStack.back().line; + } + ret.inconclusive = _inconclusive; + ret.symbolNames = _symbolNames; + return ret; +} + + std::string ErrorLogger::ErrorMessage::serialize() const { // Serialize this message into a simple string @@ -378,6 +406,20 @@ std::string ErrorLogger::ErrorMessage::toXML() const printer.PushAttribute("info", it->getinfo().c_str()); printer.CloseElement(false); } + for (std::string::size_type pos = 0; pos < _symbolNames.size();) { + const std::string::size_type pos2 = _symbolNames.find('\n', pos); + std::string symbolName; + if (pos2 == std::string::npos) { + symbolName = _symbolNames.substr(pos); + pos = pos2; + } else { + symbolName = _symbolNames.substr(pos, pos2-pos); + pos = pos2 + 1; + } + printer.OpenElement("symbol", false); + printer.PushText(symbolName.c_str()); + printer.CloseElement(false); + } printer.CloseElement(false); return printer.CStr(); } @@ -470,20 +512,20 @@ std::string ErrorLogger::ErrorMessage::toString(bool verbose, const std::string } } -void ErrorLogger::reportUnmatchedSuppressions(const std::list &unmatched) +void ErrorLogger::reportUnmatchedSuppressions(const std::list &unmatched) { // Report unmatched suppressions - for (std::list::const_iterator i = unmatched.begin(); i != unmatched.end(); ++i) { + for (std::list::const_iterator i = unmatched.begin(); i != unmatched.end(); ++i) { // don't report "unmatchedSuppression" as unmatched - if (i->id == "unmatchedSuppression") + if (i->errorId == "unmatchedSuppression") continue; // check if this unmatched suppression is suppressed bool suppressed = false; - for (std::list::const_iterator i2 = unmatched.begin(); i2 != unmatched.end(); ++i2) { - if (i2->id == "unmatchedSuppression") { - if ((i2->file == "*" || i2->file == i->file) && - (i2->line == 0 || i2->line == i->line)) { + for (std::list::const_iterator i2 = unmatched.begin(); i2 != unmatched.end(); ++i2) { + if (i2->errorId == "unmatchedSuppression") { + if ((i2->fileName == "*" || i2->fileName == i->fileName) && + (i2->lineNumber == Suppressions::Suppression::NO_LINE || i2->lineNumber == i->lineNumber)) { suppressed = true; break; } @@ -493,8 +535,10 @@ void ErrorLogger::reportUnmatchedSuppressions(const std::list callStack = { ErrorLogger::ErrorMessage::FileLocation(i->file, i->line) }; - reportErr(ErrorLogger::ErrorMessage(callStack, emptyString, Severity::information, "Unmatched suppression: " + i->id, "unmatchedSuppression", false)); + std::list callStack; + if (!i->fileName.empty()) + callStack.push_back(ErrorLogger::ErrorMessage::FileLocation(i->fileName, i->lineNumber)); + reportErr(ErrorLogger::ErrorMessage(callStack, emptyString, Severity::information, "Unmatched suppression: " + i->errorId, "unmatchedSuppression", false)); } } diff --git a/lib/errorlogger.h b/lib/errorlogger.h index a778a91d2..9851e0750 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -288,6 +288,13 @@ public: return _verboseMessage; } + /** Symbol names */ + const std::string &symbolNames() const { + return _symbolNames; + } + + Suppressions::ErrorMessage toSuppressionsErrorMessage() const; + private: /** * Replace all occurrences of searchFor with replaceWith in the @@ -305,6 +312,9 @@ public: /** Verbose message */ std::string _verboseMessage; + + /** symbol names */ + std::string _symbolNames; }; ErrorLogger() { } @@ -355,7 +365,7 @@ public: * Report list of unmatched suppressions * @param unmatched list of unmatched suppressions (from Settings::Suppressions::getUnmatched(Local|Global)Suppressions) */ - void reportUnmatchedSuppressions(const std::list &unmatched); + void reportUnmatchedSuppressions(const std::list &unmatched); static std::string callStackToString(const std::list &callStack); diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index ac47679f5..e0ee62d70 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -101,7 +101,7 @@ static void inlineSuppressions(const simplecpp::TokenList &tokens, Settings &_se // Add the suppressions. for (std::list::const_iterator it = suppressionIDs.begin(); it != suppressionIDs.end(); ++it) { - _settings.nomsg.addSuppression(*it, relativeFilename, tok->location.line); + _settings.nomsg.addSuppression(Suppressions::Suppression(*it, relativeFilename, tok->location.line)); } suppressionIDs.clear(); } @@ -713,9 +713,14 @@ void Preprocessor::error(const std::string &filename, unsigned int linenr, const void Preprocessor::missingInclude(const std::string &filename, unsigned int linenr, const std::string &header, HeaderTypes headerType) { const std::string fname = Path::fromNativeSeparators(filename); - if (_settings.nomsg.isSuppressed("missingInclude", fname, linenr)) + Suppressions::ErrorMessage errorMessage; + errorMessage.errorId = "missingInclude"; + errorMessage.setFileName(fname); + errorMessage.lineNumber = linenr; + if (_settings.nomsg.isSuppressed(errorMessage)) return; - if (headerType == SystemHeader && _settings.nomsg.isSuppressed("missingIncludeSystem", fname, linenr)) + errorMessage.errorId = "missingIncludeSystem"; + if (headerType == SystemHeader && _settings.nomsg.isSuppressed(errorMessage)) return; if (headerType == SystemHeader) diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index cf8d67233..eb076185e 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -18,14 +18,30 @@ #include "suppressions.h" +#include "mathlib.h" #include "path.h" +#include + #include #include // std::isdigit, std::isalnum, etc #include #include #include +static bool isValidGlobPattern(const std::string &pattern) +{ + for (std::string::const_iterator i = pattern.begin(); i != pattern.end(); ++i) { + if (*i == '*' || *i == '?') { + std::string::const_iterator j = i + 1; + if (j != pattern.end() && (*j == '*' || *j == '?')) { + return false; + } + } + } + return true; +} + std::string Suppressions::parseFile(std::istream &istr) { // Change '\r' to '\n' in the istr @@ -54,48 +70,203 @@ std::string Suppressions::parseFile(std::istream &istr) return ""; } + +std::string Suppressions::parseXmlFile(const char *filename) +{ + tinyxml2::XMLDocument doc; + tinyxml2::XMLError error = doc.LoadFile(filename); + if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND) + return "File not found"; + + const tinyxml2::XMLElement * const rootnode = doc.FirstChildElement(); + for (const tinyxml2::XMLElement * e = rootnode->FirstChildElement(); e; e = e->NextSiblingElement()) { + if (std::strcmp(e->Name(), "suppress") == 0) { + Suppression s; + for (const tinyxml2::XMLElement * e2 = e->FirstChildElement(); e2; e2 = e2->NextSiblingElement()) { + const char *text = e2->GetText() ? e2->GetText() : ""; + if (std::strcmp(e2->Name(), "id") == 0) + s.errorId = text; + else if (std::strcmp(e2->Name(), "fileName") == 0) + s.fileName = text; + else if (std::strcmp(e2->Name(), "lineNumber") == 0) + s.lineNumber = std::atoi(text); + else if (std::strcmp(e2->Name(), "symbolName") == 0) + s.symbolName = text; + } + const std::string err = addSuppression(s); + if (!err.empty()) + return err; + } + } + + return ""; +} + std::string Suppressions::addSuppressionLine(const std::string &line) { std::istringstream lineStream(line); - std::string id; - std::string file; - unsigned int lineNumber = 0; - if (std::getline(lineStream, id, ':')) { - if (std::getline(lineStream, file)) { + Suppressions::Suppression suppression; + if (std::getline(lineStream, suppression.errorId, ':')) { + if (std::getline(lineStream, suppression.fileName)) { // If there is not a dot after the last colon in "file" then // the colon is a separator and the contents after the colon // is a line number.. // Get position of last colon - const std::string::size_type pos = file.rfind(':'); + const std::string::size_type pos = suppression.fileName.rfind(':'); // if a colon is found and there is no dot after it.. if (pos != std::string::npos && - file.find('.', pos) == std::string::npos) { + suppression.fileName.find('.', pos) == std::string::npos) { // Try to parse out the line number try { - std::istringstream istr1(file.substr(pos+1)); - istr1 >> lineNumber; + std::istringstream istr1(suppression.fileName.substr(pos+1)); + istr1 >> suppression.lineNumber; } catch (...) { - lineNumber = 0; + suppression.lineNumber = 0; } - if (lineNumber > 0) { - file.erase(pos); + if (suppression.lineNumber > 0) { + suppression.fileName.erase(pos); } } } } - // We could perhaps check if the id is valid and return error if it is not - const std::string errmsg(addSuppression(id, Path::fromNativeSeparators(file), lineNumber)); - if (!errmsg.empty()) - return errmsg; + suppression.fileName = Path::simplifyPath(suppression.fileName); + + return addSuppression(suppression); +} + +std::string Suppressions::addSuppression(const Suppressions::Suppression &suppression) +{ + // Check that errorId is valid.. + if (suppression.errorId.empty()) { + return "Failed to add suppression. No id."; + } + if (suppression.errorId != "*") { + for (std::string::size_type pos = 0; pos < suppression.errorId.length(); ++pos) { + if (suppression.errorId[pos] < 0 || (!std::isalnum(suppression.errorId[pos]) && suppression.errorId[pos] != '_')) { + return "Failed to add suppression. Invalid id \"" + suppression.errorId + "\""; + } + if (pos == 0 && std::isdigit(suppression.errorId[pos])) { + return "Failed to add suppression. Invalid id \"" + suppression.errorId + "\""; + } + } + } + + if (!isValidGlobPattern(suppression.errorId)) + return "Failed to add suppression. Invalid glob pattern '" + suppression.errorId + "'."; + if (!isValidGlobPattern(suppression.fileName)) + return "Failed to add suppression. Invalid glob pattern '" + suppression.fileName + "'."; + + _suppressions.push_back(suppression); return ""; } -bool Suppressions::FileMatcher::match(const std::string &pattern, const std::string &name) +void Suppressions::ErrorMessage::setFileName(const std::string &s) +{ + _fileName = Path::simplifyPath(s); +} + +bool Suppressions::Suppression::isSuppressed(const Suppressions::ErrorMessage &errmsg) const +{ + if (!errorId.empty() && !matchglob(errorId, errmsg.errorId)) + return false; + if (!fileName.empty() && !matchglob(fileName, errmsg.getFileName())) + return false; + if (lineNumber > 0 && lineNumber != errmsg.lineNumber) + return false; + if (!symbolName.empty()) { + for (std::string::size_type pos = 0; pos < errmsg.symbolNames.size();) { + const std::string::size_type pos2 = errmsg.symbolNames.find('\n',pos); + std::string symname; + if (pos2 == std::string::npos) { + symname = errmsg.symbolNames.substr(pos); + pos = pos2; + } else { + symname = errmsg.symbolNames.substr(pos,pos2-pos); + pos = pos2+1; + } + if (matchglob(symbolName, symname)) + return true; + } + return false; + } + return true; +} + +bool Suppressions::Suppression::isMatch(const Suppressions::ErrorMessage &errmsg) +{ + if (!isSuppressed(errmsg)) + return false; + matched = true; + return true; +} + +std::string Suppressions::Suppression::getText() const +{ + std::string ret; + if (!errorId.empty()) + ret = errorId; + if (!fileName.empty()) + ret += " fileName=" + fileName; + if (lineNumber > 0) + ret += " lineNumber=" + MathLib::toString(lineNumber); + if (!symbolName.empty()) + ret += " symbolName=" + symbolName; + if (ret.compare(0,1," ")==0) + return ret.substr(1); + return ret; +} + +bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg) +{ + const bool unmatchedSuppression(errmsg.errorId == "unmatchedSuppression"); + for (std::list::iterator it = _suppressions.begin(); it != _suppressions.end(); ++it) { + Suppression &s = *it; + if (unmatchedSuppression && s.errorId != errmsg.errorId) + continue; + if (s.isMatch(errmsg)) + return true; + } + return false; +} + +std::list Suppressions::getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const +{ + std::list result; + for (std::list::const_iterator it = _suppressions.begin(); it != _suppressions.end(); ++it) { + const Suppression &s = *it; + if (s.matched) + continue; + if (!unusedFunctionChecking && s.errorId == "unusedFunction") + continue; + if (!file.empty() && !s.fileName.empty() && s.fileName != file) + continue; + result.push_back(s); + } + return result; +} + +std::list Suppressions::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const +{ + std::list result; + for (std::list::const_iterator it = _suppressions.begin(); it != _suppressions.end(); ++it) { + const Suppression &s = *it; + if (s.matched) + continue; + if (!unusedFunctionChecking && s.errorId == "unusedFunction") + continue; + if (s.fileName.find_first_of("?*") == std::string::npos) + continue; + result.push_back(s); + } + return result; +} + +bool Suppressions::matchglob(const std::string &pattern, const std::string &name) { const char *p = pattern.c_str(); const char *n = name.c_str(); @@ -107,17 +278,17 @@ bool Suppressions::FileMatcher::match(const std::string &pattern, const std::str switch (*p) { case '*': // Step forward until we match the next character after * - while (*n != '\0' && *n != p[1]) { + while (*n != '\0' && *n != p[1] && *n != '\\' && *n != '/') { n++; } - if (*n != '\0') { + if (*n != '\0' && *n != '/') { // If this isn't the last possibility, save it for later backtrack.push(std::make_pair(p, n)); } break; case '?': // Any character matches unless we're at the end of the name - if (*n != '\0') { + if (*n != '\0' && *n != '\\' && *n != '/') { n++; } else { matching = false; @@ -127,6 +298,10 @@ bool Suppressions::FileMatcher::match(const std::string &pattern, const std::str // Non-wildcard characters match literally if (*n == *p) { n++; + } else if (*n == '\\' && *p == '/') { + n++; + } else if (*n == '/' && *p == '\\') { + n++; } else { matching = false; } @@ -154,149 +329,3 @@ bool Suppressions::FileMatcher::match(const std::string &pattern, const std::str n++; } } - -std::string Suppressions::FileMatcher::addFile(const std::string &name, unsigned int line) -{ - if (name.find_first_of("*?") != std::string::npos) { - for (std::string::const_iterator i = name.begin(); i != name.end(); ++i) { - if (*i == '*') { - const std::string::const_iterator j = i + 1; - if (j != name.end() && (*j == '*' || *j == '?')) { - return "Failed to add suppression. Syntax error in glob."; - } - } - } - _globs[name][line] = false; - } else if (name.empty()) { - _globs["*"][0U] = false; - } else { - _files[Path::simplifyPath(name)][line] = false; - } - return ""; -} - -bool Suppressions::FileMatcher::isSuppressed(const std::string &file, unsigned int line) -{ - if (isSuppressedLocal(file, line)) - return true; - - for (std::map >::iterator g = _globs.begin(); g != _globs.end(); ++g) { - if (match(g->first, file)) { - std::map::iterator l = g->second.find(0U); - if (l != g->second.end()) { - l->second = true; - return true; - } - l = g->second.find(line); - if (l != g->second.end()) { - l->second = true; - return true; - } - } - } - - return false; -} - -bool Suppressions::FileMatcher::isSuppressedLocal(const std::string &file, unsigned int line) -{ - std::map >::iterator f = _files.find(Path::fromNativeSeparators(file)); - if (f != _files.end()) { - std::map::iterator l = f->second.find(0U); - if (l != f->second.end()) { - l->second = true; - return true; - } - l = f->second.find(line); - if (l != f->second.end()) { - l->second = true; - return true; - } - } - - return false; -} - -std::string Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line) -{ - // Check that errorId is valid.. - if (errorId.empty()) { - return "Failed to add suppression. No id."; - } - if (errorId != "*") { - for (std::string::size_type pos = 0; pos < errorId.length(); ++pos) { - if (errorId[pos] < 0 || (!std::isalnum(errorId[pos]) && errorId[pos] != '_')) { - return "Failed to add suppression. Invalid id \"" + errorId + "\""; - } - if (pos == 0 && std::isdigit(errorId[pos])) { - return "Failed to add suppression. Invalid id \"" + errorId + "\""; - } - } - } - - return _suppressions[errorId].addFile(file, line); -} - -bool Suppressions::isSuppressed(const std::string &errorId, const std::string &file, unsigned int line) -{ - if (errorId != "unmatchedSuppression" && _suppressions.find("*") != _suppressions.end()) - if (_suppressions["*"].isSuppressed(file, line)) - return true; - - std::map::iterator suppression = _suppressions.find(errorId); - if (suppression == _suppressions.end()) - return false; - - return suppression->second.isSuppressed(file, line); -} - -bool Suppressions::isSuppressedLocal(const std::string &errorId, const std::string &file, unsigned int line) -{ - if (errorId != "unmatchedSuppression" && _suppressions.find("*") != _suppressions.end()) - if (_suppressions["*"].isSuppressedLocal(file, line)) - return true; - - std::map::iterator suppression = _suppressions.find(errorId); - if (suppression == _suppressions.end()) - return false; - - return suppression->second.isSuppressedLocal(file, line); -} - -std::list Suppressions::getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const -{ - std::list result; - for (std::map::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) { - if (!unusedFunctionChecking && i->first == "unusedFunction") - continue; - - const std::map >::const_iterator f = i->second._files.find(Path::fromNativeSeparators(file)); - if (f != i->second._files.end()) { - for (std::map::const_iterator l = f->second.begin(); l != f->second.end(); ++l) { - if (!l->second) { - result.push_back(SuppressionEntry(i->first, f->first, l->first)); - } - } - } - } - return result; -} - -std::list Suppressions::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const -{ - std::list result; - for (std::map::const_iterator i = _suppressions.begin(); i != _suppressions.end(); ++i) { - if (!unusedFunctionChecking && i->first == "unusedFunction") - continue; - - // global suppressions.. - for (std::map >::const_iterator g = i->second._globs.begin(); g != i->second._globs.end(); ++g) { - for (std::map::const_iterator l = g->second.begin(); l != g->second.end(); ++l) { - if (!l->second) { - result.push_back(SuppressionEntry(i->first, g->first, l->first)); - } - } - } - } - return result; -} diff --git a/lib/suppressions.h b/lib/suppressions.h index e373aa4d9..083e4d099 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -24,7 +24,7 @@ #include #include -#include +#include #include /// @addtogroup Core @@ -32,52 +32,62 @@ /** @brief class for handling suppressions */ class CPPCHECKLIB Suppressions { -private: - class CPPCHECKLIB FileMatcher { - friend class Suppressions; +public: + + struct CPPCHECKLIB ErrorMessage { + std::string errorId; + void setFileName(const std::string &s); + const std::string &getFileName() const { + return _fileName; + } + int lineNumber; + bool inconclusive; + std::string symbolNames; private: - /** @brief List of filenames suppressed, bool flag indicates whether suppression matched. */ - std::map > _files; - /** @brief List of globs suppressed, bool flag indicates whether suppression matched. */ - std::map > _globs; - - /** - * @brief Match a name against a glob pattern. - * @param pattern The glob pattern to match. - * @param name The filename to match against the glob pattern. - * @return match success - */ - static bool match(const std::string &pattern, const std::string &name); - - public: - /** - * @brief Add a file or glob (and line number). - * @param name File name or glob pattern - * @param line Line number - * @return error message. empty upon success - */ - std::string addFile(const std::string &name, unsigned int line); - - /** - * @brief Returns true if the file name matches a previously added file or glob pattern. - * @param file File name to check - * @param line Line number - * @return true if this filename/line matches - */ - bool isSuppressed(const std::string &file, unsigned int line); - - /** - * @brief Returns true if the file name matches a previously added file (only, not glob pattern). - * @param file File name to check - * @param line Line number - * @return true if this filename/line matches - */ - bool isSuppressedLocal(const std::string &file, unsigned int line); + std::string _fileName; + }; + + struct CPPCHECKLIB Suppression { + Suppression() : lineNumber(NO_LINE), matched(false) {} + Suppression(const Suppression &other) { + *this = other; + } + Suppression(const std::string &id, const std::string &file, int line=NO_LINE) : errorId(id), fileName(file), lineNumber(line), matched(false) {} + + Suppression & operator=(const Suppression &other) { + errorId = other.errorId; + fileName = other.fileName; + lineNumber = other.lineNumber; + symbolName = other.symbolName; + matched = other.matched; + return *this; + } + + bool operator<(const Suppression &other) const { + if (errorId != other.errorId) + return errorId < other.errorId; + if (lineNumber < other.lineNumber) + return lineNumber < other.lineNumber; + if (fileName != other.fileName) + return fileName < other.fileName; + if (symbolName != other.symbolName) + return symbolName < other.symbolName; + }; + + bool isSuppressed(const ErrorMessage &errmsg) const; + + bool isMatch(const ErrorMessage &errmsg); + std::string getText() const; + + std::string errorId; + std::string fileName; + int lineNumber; + std::string symbolName; + bool matched; + + static const int NO_LINE = 0; }; - /** @brief List of error which the user doesn't want to see. */ - std::map _suppressions; -public: /** * @brief Don't show errors listed in the file. * @param istr Open file stream where errors can be read. @@ -85,6 +95,13 @@ public: */ std::string parseFile(std::istream &istr); + /** + * @brief Don't show errors listed in the file. + * @param filename file name + * @return error message. empty upon success + */ + std::string parseXmlFile(const char *filename); + /** * @brief Don't show the given error. * @param line Description of error to suppress (in id:file:line format). @@ -100,47 +117,31 @@ public: * @param line number, e.g. "123" * @return error message. empty upon success */ - std::string addSuppression(const std::string &errorId, const std::string &file = emptyString, unsigned int line = 0); + std::string addSuppression(const Suppression &suppression); /** * @brief Returns true if this message should not be shown to the user. - * @param errorId the id for the error, e.g. "arrayIndexOutOfBounds" - * @param file File name with the path, e.g. "src/main.cpp" - * @param line number, e.g. "123" + * @param errmsg error message * @return true if this error is suppressed. */ - bool isSuppressed(const std::string &errorId, const std::string &file, unsigned int line); - - /** - * @brief Returns true if this message should not be shown to the user (explicit files only, not glob patterns). - * @param errorId the id for the error, e.g. "arrayIndexOutOfBounds" - * @param file File name with the path, e.g. "src/main.cpp" - * @param line number, e.g. "123" - * @return true if this error is suppressed. - */ - bool isSuppressedLocal(const std::string &errorId, const std::string &file, unsigned int line); - - struct SuppressionEntry { - SuppressionEntry(const std::string &aid, const std::string &afile, unsigned int aline) - : id(aid), file(afile), line(aline) { - } - - std::string id; - std::string file; - unsigned int line; - }; + bool isSuppressed(const ErrorMessage &errmsg); /** * @brief Returns list of unmatched local (per-file) suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const; + std::list getUnmatchedLocalSuppressions(const std::string &file, const bool unusedFunctionChecking) const; /** * @brief Returns list of unmatched global (glob pattern) suppressions. * @return list of unmatched suppressions */ - std::list getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const; + std::list getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const; + + static bool matchglob(const std::string &pattern, const std::string &name); +private: + /** @brief List of error which the user doesn't want to see. */ + std::list _suppressions; }; /// @} diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 56c914260..77633fe1e 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -757,12 +757,20 @@ private: } } + static Suppressions::ErrorMessage errorMessage(const std::string &errorId, const std::string &fileName, int lineNumber) { + Suppressions::ErrorMessage e; + e.errorId = errorId; + e.setFileName(fileName); + e.lineNumber = lineNumber; + return e; + } + void suppressionSingle() { REDIRECT; const char *argv[] = {"cppcheck", "--suppress=uninitvar", "file.cpp"}; settings = Settings(); ASSERT(defParser.ParseFromArgs(3, argv)); - ASSERT_EQUALS(true, settings.nomsg.isSuppressed("uninitvar", "file.cpp", 1U)); + ASSERT_EQUALS(true, settings.nomsg.isSuppressed(errorMessage("uninitvar", "file.cpp", 1))); } void suppressionSingleFile() { @@ -770,7 +778,7 @@ private: const char *argv[] = {"cppcheck", "--suppress=uninitvar:file.cpp", "file.cpp"}; settings = Settings(); ASSERT(defParser.ParseFromArgs(3, argv)); - ASSERT_EQUALS(true, settings.nomsg.isSuppressed("uninitvar", "file.cpp", 1U)); + ASSERT_EQUALS(true, settings.nomsg.isSuppressed(errorMessage("uninitvar", "file.cpp", 1U))); } void suppressionTwo() { @@ -778,8 +786,8 @@ private: const char *argv[] = {"cppcheck", "--suppress=uninitvar,noConstructor", "file.cpp"}; settings = Settings(); TODO_ASSERT_EQUALS(true, false, defParser.ParseFromArgs(3, argv)); - TODO_ASSERT_EQUALS(true, false, settings.nomsg.isSuppressed("uninitvar", "file.cpp", 1U)); - TODO_ASSERT_EQUALS(true, false, settings.nomsg.isSuppressed("noConstructor", "file.cpp", 1U)); + TODO_ASSERT_EQUALS(true, false, settings.nomsg.isSuppressed(errorMessage("uninitvar", "file.cpp", 1U))); + TODO_ASSERT_EQUALS(true, false, settings.nomsg.isSuppressed(errorMessage("noConstructor", "file.cpp", 1U))); } void suppressionTwoSeparate() { @@ -787,8 +795,8 @@ private: const char *argv[] = {"cppcheck", "--suppress=uninitvar", "--suppress=noConstructor", "file.cpp"}; settings = Settings(); ASSERT(defParser.ParseFromArgs(4, argv)); - ASSERT_EQUALS(true, settings.nomsg.isSuppressed("uninitvar", "file.cpp", 1U)); - ASSERT_EQUALS(true, settings.nomsg.isSuppressed("noConstructor", "file.cpp", 1U)); + ASSERT_EQUALS(true, settings.nomsg.isSuppressed(errorMessage("uninitvar", "file.cpp", 1U))); + ASSERT_EQUALS(true, settings.nomsg.isSuppressed(errorMessage("noConstructor", "file.cpp", 1U))); } void templates() { diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index 41e1921b5..c27de54ac 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -297,7 +297,7 @@ private: } void suppressUnmatchedSuppressions() { - std::list suppressions; + std::list suppressions; // No unmatched suppression errout.str(""); @@ -308,40 +308,40 @@ private: // suppress all unmatchedSuppression errout.str(""); suppressions.clear(); - suppressions.push_back(Suppressions::SuppressionEntry("abc", "a.c", 10U)); - suppressions.push_back(Suppressions::SuppressionEntry("unmatchedSuppression", "*", 0U)); + suppressions.push_back(Suppressions::Suppression("abc", "a.c", 10U)); + suppressions.push_back(Suppressions::Suppression("unmatchedSuppression", "*", 0U)); reportUnmatchedSuppressions(suppressions); ASSERT_EQUALS("", errout.str()); // suppress all unmatchedSuppression in a.c errout.str(""); suppressions.clear(); - suppressions.push_back(Suppressions::SuppressionEntry("abc", "a.c", 10U)); - suppressions.push_back(Suppressions::SuppressionEntry("unmatchedSuppression", "a.c", 0U)); + suppressions.push_back(Suppressions::Suppression("abc", "a.c", 10U)); + suppressions.push_back(Suppressions::Suppression("unmatchedSuppression", "a.c", 0U)); reportUnmatchedSuppressions(suppressions); ASSERT_EQUALS("", errout.str()); // suppress unmatchedSuppression in a.c at line 10 errout.str(""); suppressions.clear(); - suppressions.push_back(Suppressions::SuppressionEntry("abc", "a.c", 10U)); - suppressions.push_back(Suppressions::SuppressionEntry("unmatchedSuppression", "a.c", 10U)); + suppressions.push_back(Suppressions::Suppression("abc", "a.c", 10U)); + suppressions.push_back(Suppressions::Suppression("unmatchedSuppression", "a.c", 10U)); reportUnmatchedSuppressions(suppressions); ASSERT_EQUALS("", errout.str()); // don't suppress unmatchedSuppression when file is mismatching errout.str(""); suppressions.clear(); - suppressions.push_back(Suppressions::SuppressionEntry("abc", "a.c", 10U)); - suppressions.push_back(Suppressions::SuppressionEntry("unmatchedSuppression", "b.c", 0U)); + suppressions.push_back(Suppressions::Suppression("abc", "a.c", 10U)); + suppressions.push_back(Suppressions::Suppression("unmatchedSuppression", "b.c", 0U)); reportUnmatchedSuppressions(suppressions); ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); // don't suppress unmatchedSuppression when line is mismatching errout.str(""); suppressions.clear(); - suppressions.push_back(Suppressions::SuppressionEntry("abc", "a.c", 10U)); - suppressions.push_back(Suppressions::SuppressionEntry("unmatchedSuppression", "a.c", 1U)); + suppressions.push_back(Suppressions::Suppression("abc", "a.c", 10U)); + suppressions.push_back(Suppressions::Suppression("unmatchedSuppression", "a.c", 1U)); reportUnmatchedSuppressions(suppressions); ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); } diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index d6bf9a0c9..39a1453e5 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -53,7 +53,11 @@ private: TEST_CASE(suppressingSyntaxErrors); // #7076 TEST_CASE(suppressingSyntaxErrorsInline); // #5917 + TEST_CASE(symbol); + TEST_CASE(unusedFunction); + + TEST_CASE(matchglob); } void suppressionsBadId1() const { @@ -65,21 +69,35 @@ private: ASSERT_EQUALS("", suppressions.parseFile(s2)); } + Suppressions::ErrorMessage errorMessage(const std::string &errorId) const { + Suppressions::ErrorMessage ret; + ret.errorId = errorId; + return ret; + } + + Suppressions::ErrorMessage errorMessage(const std::string &errorId, const std::string &file, int line) const { + Suppressions::ErrorMessage ret; + ret.errorId = errorId; + ret.setFileName(file); + ret.lineNumber = line; + return ret; + } + void suppressionsDosFormat() const { Suppressions suppressions; std::istringstream s("abc\r\ndef\r\n"); ASSERT_EQUALS("", suppressions.parseFile(s)); - ASSERT_EQUALS(true, suppressions.isSuppressed("abc", "test.cpp", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("def", "test.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("abc"))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("def"))); } void suppressionsFileNameWithColon() const { Suppressions suppressions; std::istringstream s("errorid:c:\\foo.cpp\nerrorid:c:\\bar.cpp:12"); ASSERT_EQUALS("", suppressions.parseFile(s)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "c:/foo.cpp", 1111)); - ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "c:/bar.cpp", 10)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "c:/bar.cpp", 12)); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "c:/foo.cpp", 1111))); + ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid", "c:/bar.cpp", 10))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "c:/bar.cpp", 12))); } void suppressionsGlob() const { @@ -87,7 +105,7 @@ private: { Suppressions suppressions; std::istringstream s("errorid:**.cpp\n"); - ASSERT_EQUALS("Failed to add suppression. Syntax error in glob.", suppressions.parseFile(s)); + ASSERT_EQUALS("Failed to add suppression. Invalid glob pattern '**.cpp'.", suppressions.parseFile(s)); } // Check that globbing works @@ -95,13 +113,13 @@ private: Suppressions suppressions; std::istringstream s("errorid:x*.cpp\nerrorid:y?.cpp\nerrorid:test.c*"); ASSERT_EQUALS("", suppressions.parseFile(s)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp.cpp", 1)); - ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "abc.cpp", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "ya.cpp", 1)); - ASSERT_EQUALS(false, suppressions.isSuppressed("errorid", "y.cpp", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "test.c", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "test.cpp", 1)); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp", 1))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp.cpp", 1))); + ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid", "abc.cpp", 1))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "ya.cpp", 1))); + ASSERT_EQUALS(false, suppressions.isSuppressed(errorMessage("errorid", "y.cpp", 1))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "test.c", 1))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "test.cpp", 1))); } // Check that both a filename match and a glob match apply @@ -109,18 +127,19 @@ private: Suppressions suppressions; std::istringstream s("errorid:x*.cpp\nerrorid:xyz.cpp:1\nerrorid:a*.cpp:1\nerrorid:abc.cpp:2"); ASSERT_EQUALS("", suppressions.parseFile(s)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "xyz.cpp", 2)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "abc.cpp", 1)); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "abc.cpp", 2)); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp", 1))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "xyz.cpp", 2))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "abc.cpp", 1))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "abc.cpp", 2))); } } void suppressionsFileNameWithExtraPath() const { // Ticket #2797 Suppressions suppressions; - suppressions.addSuppression("errorid", "./a.c", 123); - ASSERT_EQUALS(true, suppressions.isSuppressed("errorid", "a.c", 123)); + suppressions.addSuppressionLine("errorid:./a.c:123"); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "a.c", 123))); + ASSERT_EQUALS(true, suppressions.isSuppressed(errorMessage("errorid", "x/../a.c", 123))); } void reportSuppressions(const Settings &settings, const std::map &files) { @@ -226,7 +245,7 @@ private: " b++;\n" "}\n", "uninitvar"); - ASSERT_EQUALS("[*]: (information) Unmatched suppression: uninitvar\n", errout.str()); + ASSERT_EQUALS("(information) Unmatched suppression: uninitvar\n", errout.str()); // suppress uninitvar for this file only (this->*check)("void f() {\n" @@ -369,17 +388,16 @@ private: } void suppressionsPathSeparator() const { - Suppressions suppressions; - suppressions.addSuppressionLine("*:test\\*"); - ASSERT_EQUALS(true, suppressions.isSuppressed("someid", "test/foo/bar.cpp", 142)); + const Suppressions::Suppression s1("*", "test/foo/*"); + ASSERT_EQUALS(true, s1.isSuppressed(errorMessage("someid", "test/foo/bar.cpp", 142))); - suppressions.addSuppressionLine("abc:include/1.h"); - ASSERT_EQUALS(true, suppressions.isSuppressed("abc", "include\\1.h", 142)); + const Suppressions::Suppression s2("abc", "include/1.h"); + ASSERT_EQUALS(true, s2.isSuppressed(errorMessage("abc", "include/1.h", 142))); } void inlinesuppress_unusedFunction() const { // #4210, #4946 - wrong report of "unmatchedSuppression" for "unusedFunction" Suppressions suppressions; - suppressions.addSuppression("unusedFunction", "test.c", 3U); + suppressions.addSuppression(Suppressions::Suppression("unusedFunction", "test.c", 3)); ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty()); ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(true).empty()); ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", false).empty()); @@ -441,9 +459,48 @@ private: ASSERT_EQUALS("", errout.str()); } + void symbol() { + Suppressions::Suppression s; + s.errorId = "foo"; + s.symbolName = "array*"; + + Suppressions::ErrorMessage errmsg; + errmsg.errorId = "foo"; + errmsg.setFileName("test.cpp"); + errmsg.lineNumber = 123; + errmsg.symbolNames = ""; + ASSERT_EQUALS(false, s.isSuppressed(errmsg)); + errmsg.symbolNames = "x\n"; + ASSERT_EQUALS(false, s.isSuppressed(errmsg)); + errmsg.symbolNames = "array1\n"; + ASSERT_EQUALS(true, s.isSuppressed(errmsg)); + errmsg.symbolNames = "x\narray2\n"; + ASSERT_EQUALS(true, s.isSuppressed(errmsg)); + errmsg.symbolNames = "array3\nx\n"; + ASSERT_EQUALS(true, s.isSuppressed(errmsg)); + + } + void unusedFunction() { ASSERT_EQUALS(0, checkSuppression("void f() {}", "unusedFunction")); } + + void matchglob() { + ASSERT_EQUALS(true, Suppressions::matchglob("*", "xyz")); + ASSERT_EQUALS(true, Suppressions::matchglob("x*", "xyz")); + ASSERT_EQUALS(true, Suppressions::matchglob("*z", "xyz")); + ASSERT_EQUALS(true, Suppressions::matchglob("*y*", "xyz")); + ASSERT_EQUALS(true, Suppressions::matchglob("*y*", "yz")); + ASSERT_EQUALS(false, Suppressions::matchglob("*y*", "abc")); + ASSERT_EQUALS(false, Suppressions::matchglob("*", "x/y/z")); + ASSERT_EQUALS(true, Suppressions::matchglob("*/y/z", "x/y/z")); + + ASSERT_EQUALS(false, Suppressions::matchglob("?", "xyz")); + ASSERT_EQUALS(false, Suppressions::matchglob("x?", "xyz")); + ASSERT_EQUALS(false, Suppressions::matchglob("?z", "xyz")); + ASSERT_EQUALS(true, Suppressions::matchglob("?y?", "xyz")); + ASSERT_EQUALS(true, Suppressions::matchglob("?/?/?", "x/y/z")); + } }; REGISTER_TEST(TestSuppressions) diff --git a/test/testutils.h b/test/testutils.h index 65195f495..1e47e763e 100644 --- a/test/testutils.h +++ b/test/testutils.h @@ -54,7 +54,7 @@ public: _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)) + if (!msg._callStack.empty() && !_settings.nomsg.isSuppressed(msg.toSuppressionsErrorMessage())) _next->reportErr(msg); } private: