Add tools/report-clang-tidy.sh

- Fix various clang-tidy warnings
- Fix use-after-free crash when deleting profiles

22 files changed, 86 insertions, 38 deletions

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<void(QStringList &)> callback) const
+void BookmarksWidget::search(const QString &term, const std::function<void(QStringList &)> &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<void(QStringList &)> callback) const;
+    void search(const QString &term, const std::function<void(QStringList &)> &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<void(WebProfile *)> callback, WebProfile *current, bool checkable) const
+void WebProfileManager::profileMenu(QMenu *menu, const std::function<void(WebProfile *)> &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<void(WebProfile *)> callback, WebProfile *current = nullptr, bool checkable = false) const;
+    void profileMenu(QMenu *menu, const std::function<void(WebProfile *)> &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<QPluginLoader *> plugins)
     // bookmarks
     m_bookmarks = std::make_shared<BookmarksWidget>(QString::fromStdString(m_config->value<std::string>("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<Configuration> m_config;
     std::shared_ptr<BookmarksWidget> m_bookmarks;
     std::unique_ptr<DownloadsWidget> m_downloads;
-    WebProfileManager *m_profileManager;
+    WebProfileManager *m_profileManager = nullptr;
     QVector<UrlFilter *> m_filters;
 
     QVector<MainWindow *> 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<QString>("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<Configuration> &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<Configuration> &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 <functional>
 #include <QVBoxLayout>
 
-inline void run_if(SubWindow *_subwindow, std::function<void(SubWindow*, int)> f)
+inline void run_if(SubWindow *_subwindow, const std::function<void(SubWindow*, int)> &f)
 {
     if(_subwindow != nullptr)
         f(_subwindow, _subwindow->currentTabIndex());
@@ -111,7 +111,7 @@ MenuBar::MenuBar(const Configuration *config, MainWindow *parent)
         const QString sessionPath = config->value<QString>("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<SessionForm *>(ui->listWidget->itemWidget(currentItem));
-        if(currentWidget)
+        if(currentWidget != nullptr)
             currentWidget->ui->delete_toolButton->show();
 
         auto *previousWidget = qobject_cast<SessionForm *>(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<SessionForm *>(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<QJsonObject> SessionDialog::pickSession()
     }
 
     auto *currentWidget = qobject_cast<SessionForm *>(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
--- a/src/webengine/webengineprofile.cpp
+++ /dev/null
diff --git a/src/webengine/webengineprofile.h b/src/webengine/webengineprofile.h
deleted file mode 100644
index e69de29..0000000
--- a/src/webengine/webengineprofile.h
+++ /dev/null
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
+