From 9e73d0dac0774955348a5164087363c5b33927b8 Mon Sep 17 00:00:00 2001 From: Aqua-sama Date: Fri, 18 Jan 2019 16:56:54 +0100 Subject: Add tools/report-clang-tidy.sh - Fix various clang-tidy warnings - Fix use-after-free crash when deleting profiles --- .clang-tidy | 30 ++++++++++++++++++++++++++++++ lib/about/aboutplugin.cpp | 2 +- lib/bookmarks/bookmarkswidget.cpp | 2 +- lib/bookmarks/bookmarkswidget.h | 2 +- lib/bookmarks/xbel.h | 2 +- lib/urlfilter/hostlist/hostlist.cpp | 4 ++-- lib/webprofile/webprofilemanager.cpp | 5 +++-- lib/webprofile/webprofilemanager.h | 2 +- src/browser.cpp | 3 ++- src/browser.h | 2 +- src/builtins.cpp | 6 +++--- src/builtins.h | 8 ++++---- src/main.cpp | 2 +- src/mainwindow/mainwindow.cpp | 16 ++++++++-------- src/mainwindow/menubar.cpp | 4 ++-- src/session/session.cpp | 2 +- src/session/sessiondialog.cpp | 8 ++++---- src/webengine/webengineprofile.cpp | 0 src/webengine/webengineprofile.h | 0 tools/cppcheck.sh | 4 ---- tools/report-clang-tidy.sh | 16 ++++++++++++++++ tools/report-cppcheck.sh | 4 ++++ 22 files changed, 86 insertions(+), 38 deletions(-) create mode 100644 .clang-tidy delete mode 100644 src/webengine/webengineprofile.cpp delete mode 100644 src/webengine/webengineprofile.h delete mode 100755 tools/cppcheck.sh create mode 100755 tools/report-clang-tidy.sh create mode 100755 tools/report-cppcheck.sh diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..5813ee3 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,30 @@ +--- +Checks: 'clang-diagnostic-*,clang-analyzer-*,bugprone-*,cert-*,cppcoreguidelines-*,hicpp-*,misc-*,modernize-*,performance-*,portability-*,readability-*' +WarningsAsErrors: '' +HeaderFilterRegex: '' +AnalyzeTemporaryDtors: false +FormatStyle: none +User: aqua +CheckOptions: + - key: google-readability-braces-around-statements.ShortStatementLines + value: '1' + - key: google-readability-function-size.StatementThreshold + value: '800' + - key: google-readability-namespace-comments.ShortNamespaceLines + value: '10' + - key: google-readability-namespace-comments.SpacesBeforeComments + value: '2' + - key: modernize-loop-convert.MaxCopySize + value: '16' + - key: modernize-loop-convert.MinConfidence + value: reasonable + - key: modernize-loop-convert.NamingStyle + value: CamelCase + - key: modernize-pass-by-value.IncludeStyle + value: llvm + - key: modernize-replace-auto-ptr.IncludeStyle + value: llvm + - key: modernize-use-nullptr.NullMacros + value: 'NULL' +... + diff --git a/lib/about/aboutplugin.cpp b/lib/about/aboutplugin.cpp index b2fab51..b345b75 100644 --- a/lib/about/aboutplugin.cpp +++ b/lib/about/aboutplugin.cpp @@ -30,7 +30,7 @@ QTreeWidgetItem *createItem(const QString &key, const QJsonValue &json, QTreeWid case QJsonValue::Array: item->setText(1, QString()); - for(const QJsonValue &v : json.toArray()) { + for(const auto &v : json.toArray()) { createItem(QString(), v, item); } break; diff --git a/lib/bookmarks/bookmarkswidget.cpp b/lib/bookmarks/bookmarkswidget.cpp index 045b424..f3ef4df 100644 --- a/lib/bookmarks/bookmarkswidget.cpp +++ b/lib/bookmarks/bookmarkswidget.cpp @@ -135,7 +135,7 @@ void BookmarksWidget::addBookmark(const QString &title, const QString &url) model->appendBookmark(title, url, QModelIndex()); } -void BookmarksWidget::search(const QString &term, std::function callback) const +void BookmarksWidget::search(const QString &term, const std::function &callback) const { QStringList ret = model->search(term); callback(ret); diff --git a/lib/bookmarks/bookmarkswidget.h b/lib/bookmarks/bookmarkswidget.h index f30db7d..149d2a6 100644 --- a/lib/bookmarks/bookmarkswidget.h +++ b/lib/bookmarks/bookmarkswidget.h @@ -39,7 +39,7 @@ signals: public slots: void save(); void addBookmark(const QString &title, const QString &url); - void search(const QString &term, std::function callback) const; + void search(const QString &term, const std::function &callback) const; private: Ui::BookmarksDialog *ui; diff --git a/lib/bookmarks/xbel.h b/lib/bookmarks/xbel.h index 97a6ed4..44a65bb 100644 --- a/lib/bookmarks/xbel.h +++ b/lib/bookmarks/xbel.h @@ -13,7 +13,7 @@ class QIODevice; class BookmarkItem; namespace Xbel { -void read(QIODevice *device, BookmarkItem *model); +void read(QIODevice *device, BookmarkItem *item); void write(QIODevice *device, const BookmarkItem *item); } diff --git a/lib/urlfilter/hostlist/hostlist.cpp b/lib/urlfilter/hostlist/hostlist.cpp index ec0b214..ff652cf 100644 --- a/lib/urlfilter/hostlist/hostlist.cpp +++ b/lib/urlfilter/hostlist/hostlist.cpp @@ -62,11 +62,11 @@ void HostList::parseLine(const QString& line) return; const QStringList parts = parsedLine.split(QLatin1Literal(" ")); - const QString redirect = parts.at(0); + const QString &redirect = parts.at(0); const auto action = (redirect == QLatin1Literal("0.0.0.0")) ? UrlFilter::Block : UrlFilter::Redirect; for(int i = 1; i < parts.size(); i++) { - const QString domain = parts.at(i); + const QString &domain = parts.at(i); Rule r; r.action = action; r.domainHash = qHash(domain); diff --git a/lib/webprofile/webprofilemanager.cpp b/lib/webprofile/webprofilemanager.cpp index d22b75c..2fca224 100644 --- a/lib/webprofile/webprofilemanager.cpp +++ b/lib/webprofile/webprofilemanager.cpp @@ -27,8 +27,9 @@ WebProfileManager::~WebProfileManager() if(!p.ptr->cachePath().isEmpty()) QDir(p.ptr->cachePath()).removeRecursively(); } + const QString filename = p.settings->fileName(); delete p.settings; - QFile::remove(p.settings->fileName()); + QFile::remove(filename); } else if(p.settings != nullptr) { #ifdef QT_DEBUG qDebug("sync %s", qUtf8Printable(p.settings->fileName())); @@ -132,7 +133,7 @@ void WebProfileManager::deleteProfile(const QString &id) } } -void WebProfileManager::profileMenu(QMenu *menu, std::function callback, WebProfile *current, bool checkable) const +void WebProfileManager::profileMenu(QMenu *menu, const std::function &callback, WebProfile *current, bool checkable) const { auto *group = new QActionGroup(menu); connect(menu, &QMenu::aboutToHide, group, &QActionGroup::deleteLater); diff --git a/lib/webprofile/webprofilemanager.h b/lib/webprofile/webprofilemanager.h index 822dc7d..f7bf52a 100644 --- a/lib/webprofile/webprofilemanager.h +++ b/lib/webprofile/webprofilemanager.h @@ -39,7 +39,7 @@ public: */ void deleteProfile(const QString &id); - void profileMenu(QMenu *menu, std::function callback, WebProfile *current = nullptr, bool checkable = false) const; + void profileMenu(QMenu *menu, const std::function &callback, WebProfile *current = nullptr, bool checkable = false) const; const QStringList idList() const { diff --git a/src/browser.cpp b/src/browser.cpp index 6e1e2ba..430a7d4 100644 --- a/src/browser.cpp +++ b/src/browser.cpp @@ -216,11 +216,12 @@ void Browser::setup(QVector plugins) // bookmarks m_bookmarks = std::make_shared(QString::fromStdString(m_config->value("bookmarks.path").value())); connect(m_bookmarks.get(), &BookmarksWidget::showContextMenu, this, [this](const QUrl &url, const QPoint &pos) { - auto *menu = new QMenu(m_bookmarks.get()); auto *subwindow = m_windows.last()->currentSubWindow(); if(subwindow == nullptr) return; + auto *menu = new QMenu(m_bookmarks.get()); + menu->addAction(tr("Open link in current tab"), subwindow, [url, subwindow]() { subwindow->currentView()->load(url); }); diff --git a/src/browser.h b/src/browser.h index 32011e0..a005513 100644 --- a/src/browser.h +++ b/src/browser.h @@ -90,7 +90,7 @@ private: std::unique_ptr m_config; std::shared_ptr m_bookmarks; std::unique_ptr m_downloads; - WebProfileManager *m_profileManager; + WebProfileManager *m_profileManager = nullptr; QVector m_filters; QVector m_windows; diff --git a/src/builtins.cpp b/src/builtins.cpp index 8cfdb46..55bdc31 100644 --- a/src/builtins.cpp +++ b/src/builtins.cpp @@ -36,9 +36,9 @@ int builtins::build() } int builtins::help(const char *cmd, - boost::program_options::options_description cmd_opts, - boost::program_options::options_description config_opts, - CommandHash_t pluginCommands, const QTranslator *translator) + const boost::program_options::options_description &cmd_opts, + const boost::program_options::options_description &config_opts, + const CommandHash_t &pluginCommands, const QTranslator *translator) { const auto version = QVersionNumber::fromString(QLatin1String(poi_Version)).toString().toStdString(); std::cout << tr(translator, "smolbote ") << version << tr(translator, ": yet another no-frills browser\n"); diff --git a/src/builtins.h b/src/builtins.h index 7ad8ba5..e151e8d 100644 --- a/src/builtins.h +++ b/src/builtins.h @@ -17,10 +17,10 @@ namespace builtins int version(); int build(); int help(const char *cmd, - boost::program_options::options_description cmd_opts, - boost::program_options::options_description config_opts, - CommandHash_t pluginCommands, - const QTranslator *translator); + const boost::program_options::options_description &cmd_opts, + const boost::program_options::options_description &config_opts, + const CommandHash_t &pluginCommands, + const QTranslator *translator); } #endif diff --git a/src/main.cpp b/src/main.cpp index 11db0d7..0072b04 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -75,7 +75,7 @@ int main(int argc, char **argv) // Load plugins for(const QString &path : Util::files(config->value("plugins.path").value())) { - QPluginLoader *loader = new QPluginLoader(path); + auto *loader = new QPluginLoader(path); const bool loaded = loader->load(); #ifdef QT_DEBUG qDebug("Loading plugin %s %s", qUtf8Printable(path), loaded ? "[ok]" : "[failed]"); diff --git a/src/mainwindow/mainwindow.cpp b/src/mainwindow/mainwindow.cpp index 71b594a..7fb7d9e 100644 --- a/src/mainwindow/mainwindow.cpp +++ b/src/mainwindow/mainwindow.cpp @@ -69,7 +69,7 @@ MainWindow::MainWindow(const std::unique_ptr &config, QWidget *pa config->setShortcut(subwindowMenuAction, "subwindow.shortcuts.menu"); connect(subwindowMenuAction, &QAction::triggered, this, [this]() { QMdiSubWindow *window = mdiArea->currentSubWindow(); - if(window) { + if(window != nullptr) { // show the menu at the subwindow position // position has to be global, and mapped by the mdiArea (parentWidget() of the subwindow) const auto position = mdiArea->mapToGlobal(window->pos()); @@ -114,20 +114,20 @@ MainWindow::MainWindow(const std::unique_ptr &config, QWidget *pa // address bar connect(addressBar, &AddressBar::search, this, [this](const QString &term) { - if(this->currentView) { + if(this->currentView != nullptr) { currentView->search(term); currentView->setFocus(); } }); connect(addressBar, &AddressBar::load, this, [this](const QUrl &url) { - if(this->currentView) { + if(this->currentView != nullptr) { currentView->load(url); currentView->setFocus(); } }); connect(addressBar, &AddressBar::giveFocus, this, [this]() { - if(this->currentView) { + if(this->currentView != nullptr) { currentView->setFocus(); } }); @@ -244,13 +244,13 @@ SubWindow *MainWindow::createSubWindow(WebProfile *profile, bool openProfileNewt void MainWindow::setView(WebView *view) { - if(currentView) { + if(currentView != nullptr) { // disconnect old view - disconnect(currentView, 0, addressBar, 0); + disconnect(currentView, nullptr, addressBar, nullptr); } currentView = view; - if(view) { + if(view != nullptr) { connect(view, &WebView::urlChanged, addressBar, &AddressBar::setUrl); addressBar->setUrl(view->url()); @@ -277,7 +277,7 @@ void MainWindow::closeEvent(QCloseEvent *event) } mdiArea->closeAllSubWindows(); - if(mdiArea->currentSubWindow()) + if(mdiArea->currentSubWindow() != nullptr) event->ignore(); else event->accept(); diff --git a/src/mainwindow/menubar.cpp b/src/mainwindow/menubar.cpp index 117641f..7b7d912 100644 --- a/src/mainwindow/menubar.cpp +++ b/src/mainwindow/menubar.cpp @@ -30,7 +30,7 @@ #include #include -inline void run_if(SubWindow *_subwindow, std::function f) +inline void run_if(SubWindow *_subwindow, const std::function &f) { if(_subwindow != nullptr) f(_subwindow, _subwindow->currentTabIndex()); @@ -111,7 +111,7 @@ MenuBar::MenuBar(const Configuration *config, MainWindow *parent) const QString sessionPath = config->value("browser.session.path").value(); auto *actionSaveSession = smolbote->addAction(tr("Save Session"), parent, [sessionPath]() { auto *sessionDialog = new SaveSessionDialog(nullptr); - if(sessionDialog->exec()) + if(sessionDialog->exec() == QDialog::Accepted) sessionDialog->save(sessionPath); }); config->setShortcut(actionSaveSession, "mainwindow.shortcuts.saveSession"); diff --git a/src/session/session.cpp b/src/session/session.cpp index 51575bf..67cea70 100644 --- a/src/session/session.cpp +++ b/src/session/session.cpp @@ -120,7 +120,7 @@ void Session::restoreView(WebView *view, const QJsonObject &data) Q_CHECK_PTR(profileManager); auto *profile = profileManager->profile(data["profile"].toString()); - if(profile) + if(profile != nullptr) view->setProfile(profile); auto url = data.value("url"); diff --git a/src/session/sessiondialog.cpp b/src/session/sessiondialog.cpp index 184c44d..b734088 100644 --- a/src/session/sessiondialog.cpp +++ b/src/session/sessiondialog.cpp @@ -35,11 +35,11 @@ SessionDialog::SessionDialog(QWidget *parent) connect(ui->listWidget, &QListWidget::currentItemChanged, this, [this](QListWidgetItem *currentItem, QListWidgetItem *previousItem) { auto *currentWidget = qobject_cast(ui->listWidget->itemWidget(currentItem)); - if(currentWidget) + if(currentWidget != nullptr) currentWidget->ui->delete_toolButton->show(); auto *previousWidget = qobject_cast(ui->listWidget->itemWidget(previousItem)); - if(previousWidget) + if(previousWidget != nullptr) previousWidget->ui->delete_toolButton->hide(); }); @@ -53,7 +53,7 @@ SessionDialog::SessionDialog(QWidget *parent) accepted_connection = connect(this, &SessionDialog::accepted, this, [this, browser]() { auto *currentWidget = qobject_cast(ui->listWidget->itemWidget(ui->listWidget->currentItem())); - if(currentWidget) + if(currentWidget != nullptr) this->openSession(currentWidget->ui->label->text()); else browser->createWindow(); @@ -76,7 +76,7 @@ std::optional SessionDialog::pickSession() } auto *currentWidget = qobject_cast(ui->listWidget->itemWidget(ui->listWidget->currentItem())); - if(currentWidget) { + if(currentWidget != nullptr) { QFile json(currentWidget->ui->label->text()); if(json.open(QIODevice::ReadOnly | QIODevice::Text)) { auto doc = QJsonDocument::fromJson(json.readAll()); diff --git a/src/webengine/webengineprofile.cpp b/src/webengine/webengineprofile.cpp deleted file mode 100644 index e69de29..0000000 diff --git a/src/webengine/webengineprofile.h b/src/webengine/webengineprofile.h deleted file mode 100644 index e69de29..0000000 diff --git a/tools/cppcheck.sh b/tools/cppcheck.sh deleted file mode 100755 index 2b130ce..0000000 --- a/tools/cppcheck.sh +++ /dev/null @@ -1,4 +0,0 @@ -#!/bin/bash - -cppcheck --enable=all --verbose src/ lib/ plugins/ 2> report.txt - diff --git a/tools/report-clang-tidy.sh b/tools/report-clang-tidy.sh new file mode 100755 index 0000000..eddb703 --- /dev/null +++ b/tools/report-clang-tidy.sh @@ -0,0 +1,16 @@ +#!/bin/bash + +cp build/compile_commands.json tidy/compile_commands.json + +# https://bugs.llvm.org/show_bug.cgi?id=37315 +sed -i 's/-pipe//g' tidy/compile_commands.json +# do not scan system headers (replace -I with -isystem) +sed -i 's/\-I\/usr\/include/\-isystem\/usr\/include/g' tidy/compile_commands.json + +for folder in $(find src lib -type d) +do + if [[ $folder != *'test' ]]; then + clang-tidy -p tidy $folder/*.cpp $folder/*.h > reports/clangtidy-$(basename $folder).txt + fi +done + diff --git a/tools/report-cppcheck.sh b/tools/report-cppcheck.sh new file mode 100755 index 0000000..2e42226 --- /dev/null +++ b/tools/report-cppcheck.sh @@ -0,0 +1,4 @@ +#!/bin/bash + +cppcheck --enable=all --project=tidy/compile_commands.json 2> report.txt + -- cgit v1.2.1