From e71225268f91fe57afcd14d92a3a5307362e9aa7 Mon Sep 17 00:00:00 2001 From: iphydf Date: Thu, 16 Apr 2020 15:59:29 +0000 Subject: [PATCH] chore: Various code cleanups. * Reorder class data members and/or constructor initialisers to match, reducing confusion about when members will be initialised. * Remove (most) unused variables. Not removed: some global variables with `TODO(sudden6)` on them for using them in the future. I don't know how far into the future sudden6 wants to use them, so I left them there for now. * Distinguish different bootstrap nodes in the logs by index in the bootstrap node list. Originally, we used to log the address/port of the node we're bootstrapping to. This was removed out of privacy concerns (even though the bootstrap nodes are public). This made the logs much less useful when debugging why the client isn't connecting. Having indices makes it easier to see that different nodes are being selected, and makes it possible to determine which node was selected. * Explicitly cast unused results of Tox API functions to `void` when all we want is to know whether the function succeeds or not. * Don't try to `#include ` on Windows. It does not exist on MSVC. * Remove extra `;` after function definitions. * Remove reference indirection of QJsonValueRef, since a copy of that ref (small pointer-like object) has to be made anyway when iterating over QJsonArrays. * Make some file-scope global state `static`. * Use `nullptr` instead of `NULL`. * Add `#if DESKTOP_NOTIFICATIONS` around the code that implements desktop notifications, so it becomes a bit easier to compile everything with a single compiler command - useful for manually running static analysers. * Fix an error on MSVC where `disconnect` is looked up to be a non-static member function and the `this` capture is missing. * Consistently use `struct` and `class` tags for types. * Use references in ranged-for where it reduces copies. * Move private static data members out of the Style class and into file-local scope. There is no need for them to be in the class. Also marked them `const` where possible. * Removed unused lambda capture. * Ensure qTox can compile under NDEBUG with `-Wunused-variable` by inlining the unused variable into the `assert` that was its only target. * Minor reformatting in core_test.cpp. --- src/chatlog/content/broken.h | 2 +- src/chatlog/content/filetransferwidget.cpp | 2 - src/core/core.cpp | 4 +- src/ipc.cpp | 4 +- src/model/group.cpp | 6 +-- src/model/message.h | 2 +- src/net/bootstrapnodeupdater.cpp | 2 +- src/persistence/history.cpp | 3 -- src/platform/autorun_osx.cpp | 2 +- src/platform/camera/avfoundation.mm | 2 +- .../desktop_notifications/desktopnotify.cpp | 2 + .../desktop_notifications/desktopnotify.h | 2 + src/widget/form/genericchatform.cpp | 4 +- src/widget/form/genericchatform.h | 2 +- src/widget/form/groupchatform.h | 2 +- src/widget/form/loadhistorydialog.cpp | 1 - src/widget/form/settings/avform.cpp | 3 +- src/widget/friendwidget.cpp | 4 +- src/widget/style.cpp | 42 ++++++++++--------- src/widget/style.h | 5 --- src/widget/widget.cpp | 12 ++---- test/core/core_test.cpp | 6 +-- test/model/messageprocessor_test.cpp | 2 +- test/model/sessionchatlog_test.cpp | 2 +- test/persistence/offlinemsgengine_test.cpp | 2 +- 25 files changed, 56 insertions(+), 64 deletions(-) diff --git a/src/chatlog/content/broken.h b/src/chatlog/content/broken.h index a7b271111..ab5534494 100644 --- a/src/chatlog/content/broken.h +++ b/src/chatlog/content/broken.h @@ -38,8 +38,8 @@ public: qreal getAscent() const override; private: - QSize size; QPixmap pmap; + QSize size; }; #endif // BROKEN_H diff --git a/src/chatlog/content/filetransferwidget.cpp b/src/chatlog/content/filetransferwidget.cpp index 3a205f4c7..6f611986a 100644 --- a/src/chatlog/content/filetransferwidget.cpp +++ b/src/chatlog/content/filetransferwidget.cpp @@ -86,8 +86,6 @@ FileTransferWidget::FileTransferWidget(QWidget* parent, ToxFile file) update(); }); - CoreFile* coreFile = Core::getInstance()->getCoreFile(); - connect(ui->leftButton, &QPushButton::clicked, this, &FileTransferWidget::onLeftButtonClicked); connect(ui->rightButton, &QPushButton::clicked, this, &FileTransferWidget::onRightButtonClicked); connect(ui->previewButton, &QPushButton::clicked, this, diff --git a/src/core/core.cpp b/src/core/core.cpp index 9e473bb59..888ca305c 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -805,7 +805,7 @@ void Core::bootstrapDht() QString dhtServerAddress = dhtServer.address.toLatin1(); QString port = QString::number(dhtServer.port); QString name = dhtServer.name; - qDebug() << QString("Connecting to a bootstrap node..."); + qDebug("Connecting to bootstrap node %d", j % listSize); QByteArray address = dhtServer.address.toLatin1(); // TODO: constucting the pk via ToxId is a workaround ToxPk pk = ToxId{dhtServer.userId}.getPublicKey(); @@ -1651,7 +1651,7 @@ bool Core::hasFriendWithPublicKey(const ToxPk& publicKey) const } Tox_Err_Friend_By_Public_Key error; - uint32_t friendId = tox_friend_by_public_key(tox.get(), publicKey.getData(), &error); + (void)tox_friend_by_public_key(tox.get(), publicKey.getData(), &error); return PARSE_ERR(error); } diff --git a/src/ipc.cpp b/src/ipc.cpp index 36d4fb29b..2467b791b 100644 --- a/src/ipc.cpp +++ b/src/ipc.cpp @@ -24,8 +24,10 @@ #include #include -#include #include +#ifndef _MSC_VER +#include +#endif namespace { diff --git a/src/model/group.cpp b/src/model/group.cpp index f08d4397d..d7a715142 100644 --- a/src/model/group.cpp +++ b/src/model/group.cpp @@ -34,13 +34,13 @@ static const int MAX_GROUP_TITLE_LENGTH = 128; Group::Group(int groupId, const GroupId persistentGroupId, const QString& name, bool isAvGroupchat, const QString& selfName, ICoreGroupQuery& groupQuery, ICoreIdHandler& idHandler) - : selfName{selfName} + : groupQuery(groupQuery) + , idHandler(idHandler) + , selfName{selfName} , title{name} , toxGroupNum(groupId) , groupId{persistentGroupId} , avGroupchat{isAvGroupchat} - , groupQuery(groupQuery) - , idHandler(idHandler) { // in groupchats, we only notify on messages containing your name <-- dumb // sound notifications should be on all messages, but system popup notification diff --git a/src/model/message.h b/src/model/message.h index 0dd9863b7..13783571c 100644 --- a/src/model/message.h +++ b/src/model/message.h @@ -108,7 +108,7 @@ public: inline void disableMentions() { detectingMentions = false; - }; + } private: bool detectingMentions = false; diff --git a/src/net/bootstrapnodeupdater.cpp b/src/net/bootstrapnodeupdater.cpp index 347ec4f73..93454739a 100644 --- a/src/net/bootstrapnodeupdater.cpp +++ b/src/net/bootstrapnodeupdater.cpp @@ -185,7 +185,7 @@ QList BootstrapNodeUpdater::jsonToNodeList(const QJsonDocument& nodeL return result; } QJsonArray nodes = rootObj[jsonNodeArrayName].toArray(); - for (const auto& node : nodes) { + for (const QJsonValueRef node : nodes) { if (node.isObject()) { jsonNodeToDhtServer(node.toObject(), result); } diff --git a/src/persistence/history.cpp b/src/persistence/history.cpp index af438e178..895aba107 100644 --- a/src/persistence/history.cpp +++ b/src/persistence/history.cpp @@ -300,9 +300,6 @@ MessageState getMessageState(bool isPending, bool isBroken) * Caches mappings to speed up message saving. */ -static constexpr int NUM_MESSAGES_DEFAULT = - 100; // arbitrary number of messages loaded when not loading by date - FileDbInsertionData::FileDbInsertionData() { static int id = qRegisterMetaType(); diff --git a/src/platform/autorun_osx.cpp b/src/platform/autorun_osx.cpp index abb20f736..4c1c9cc97 100644 --- a/src/platform/autorun_osx.cpp +++ b/src/platform/autorun_osx.cpp @@ -24,7 +24,7 @@ #include #include -int state; +static int state; bool Platform::setAutorun(bool on) { diff --git a/src/platform/camera/avfoundation.mm b/src/platform/camera/avfoundation.mm index 32ff832f1..4d5ef09c5 100644 --- a/src/platform/camera/avfoundation.mm +++ b/src/platform/camera/avfoundation.mm @@ -33,7 +33,7 @@ QVector > avfoundation::getDeviceList() } uint32_t numScreens = 0; - CGGetActiveDisplayList(0, NULL, &numScreens); + CGGetActiveDisplayList(0, nullptr, &numScreens); if (numScreens > 0) { CGDirectDisplayID screens[numScreens]; CGGetActiveDisplayList(numScreens, screens, &numScreens); diff --git a/src/platform/desktop_notifications/desktopnotify.cpp b/src/platform/desktop_notifications/desktopnotify.cpp index 3ee9414f9..59b88dd38 100644 --- a/src/platform/desktop_notifications/desktopnotify.cpp +++ b/src/platform/desktop_notifications/desktopnotify.cpp @@ -17,6 +17,7 @@ along with qTox. If not, see . */ +#if DESKTOP_NOTIFICATIONS #include "desktopnotify.h" #include @@ -75,3 +76,4 @@ void DesktopNotify::notifyMessageSimple(const MessageType type) createNotification(message, {}, snoreIcon); } +#endif diff --git a/src/platform/desktop_notifications/desktopnotify.h b/src/platform/desktop_notifications/desktopnotify.h index 391598969..8c6565f9a 100644 --- a/src/platform/desktop_notifications/desktopnotify.h +++ b/src/platform/desktop_notifications/desktopnotify.h @@ -20,6 +20,7 @@ #ifndef DESKTOPNOTIFY_H #define DESKTOPNOTIFY_H +#if DESKTOP_NOTIFICATIONS #include #include @@ -52,5 +53,6 @@ private: Snore::Application snoreApp; Snore::Icon snoreIcon; }; +#endif // DESKTOP_NOTIFICATIONS #endif // DESKTOPNOTIFY_H diff --git a/src/widget/form/genericchatform.cpp b/src/widget/form/genericchatform.cpp index 45f8dd805..08327871b 100644 --- a/src/widget/form/genericchatform.cpp +++ b/src/widget/form/genericchatform.cpp @@ -1120,9 +1120,9 @@ void GenericChatForm::renderMessages(ChatLogIdx begin, ChatLogIdx end, if (onCompletion) { auto connection = std::make_shared(); *connection = connect(chatWidget, &ChatLog::workerTimeoutFinished, - [onCompletion, connection] { + [this, onCompletion, connection] { onCompletion(); - disconnect(*connection); + this->disconnect(*connection); }); } diff --git a/src/widget/form/genericchatform.h b/src/widget/form/genericchatform.h index 6f2005679..c100fdcab 100644 --- a/src/widget/form/genericchatform.h +++ b/src/widget/form/genericchatform.h @@ -54,7 +54,7 @@ class QToolButton; class QVBoxLayout; class IMessageDispatcher; -class Message; +struct Message; namespace Ui { class MainWindow; diff --git a/src/widget/form/groupchatform.h b/src/widget/form/groupchatform.h index 6329979b2..6cce22c96 100644 --- a/src/widget/form/groupchatform.h +++ b/src/widget/form/groupchatform.h @@ -33,7 +33,7 @@ class FlowLayout; class QTimer; class GroupId; class IMessageDispatcher; -class Message; +struct Message; class GroupChatForm : public GenericChatForm { diff --git a/src/widget/form/loadhistorydialog.cpp b/src/widget/form/loadhistorydialog.cpp index aceecb054..47b9aff5d 100644 --- a/src/widget/form/loadhistorydialog.cpp +++ b/src/widget/form/loadhistorydialog.cpp @@ -89,7 +89,6 @@ void LoadHistoryDialog::enableSearchMode() void LoadHistoryDialog::highlightDates(int year, int month) { - History* history = Nexus::getProfile()->getHistory(); QDate monthStart(year, month, 1); QDate monthEnd(year, month + 1, 1); diff --git a/src/widget/form/settings/avform.cpp b/src/widget/form/settings/avform.cpp index df6a3f977..bcf5f8e3e 100644 --- a/src/widget/form/settings/avform.cpp +++ b/src/widget/form/settings/avform.cpp @@ -99,7 +99,6 @@ AVForm::AVForm(IAudioControl& audio, CoreAV* coreAV, CameraSource& camera, eventsInit(); - QDesktopWidget* desktop = QApplication::desktop(); for (QScreen* qScreen : QGuiApplication::screens()) { connect(qScreen, &QScreen::geometryChanged, this, &AVForm::rescanDevices); } @@ -308,7 +307,7 @@ void AVForm::fillCameraModesComboBox() QString str; std::string pixelFormat = CameraDevice::getPixelFormatString(mode.pixel_format).toStdString(); - qDebug("width: %d, height: %d, FPS: %f, pixel format: %s\n", mode.width, mode.height, + qDebug("width: %d, height: %d, FPS: %f, pixel format: %s", mode.width, mode.height, mode.FPS, pixelFormat.c_str()); if (mode.height && mode.width) { diff --git a/src/widget/friendwidget.cpp b/src/widget/friendwidget.cpp index c03fac63f..6f65289c5 100644 --- a/src/widget/friendwidget.cpp +++ b/src/widget/friendwidget.cpp @@ -124,7 +124,7 @@ void FriendWidget::onContextMenuCalled(QContextMenuEvent* event) connect(newGroupAction, &QAction::triggered, chatroom.get(), &FriendChatroom::inviteToNewGroup); inviteMenu->addSeparator(); - for (const auto group : chatroom->getGroups()) { + for (const auto& group : chatroom->getGroups()) { const auto groupAction = inviteMenu->addAction(tr("Invite to group '%1'").arg(group.name)); connect(groupAction, &QAction::triggered, [=]() { chatroom->inviteFriend(group.group); }); } @@ -145,7 +145,7 @@ void FriendWidget::onContextMenuCalled(QContextMenuEvent* event) circleMenu->addSeparator(); - for (const auto circle : chatroom->getOtherCircles()) { + for (const auto& circle : chatroom->getOtherCircles()) { QAction* action = new QAction(tr("Move to circle \"%1\"").arg(circle.name), circleMenu); connect(action, &QAction::triggered, [=]() { moveToCircle(circle.circleId); }); circleMenu->addAction(action); diff --git a/src/widget/style.cpp b/src/widget/style.cpp index 3d09b2195..f902002f0 100644 --- a/src/widget/style.cpp +++ b/src/widget/style.cpp @@ -89,7 +89,7 @@ static QMap dictColor; static QMap dictFont; static QMap dictTheme; -QList Style::themeNameColors = { +static const QList themeNameColors = { {Style::Light, QObject::tr("Default"), QColor()}, {Style::Light, QObject::tr("Blue"), QColor("#004aa4")}, {Style::Light, QObject::tr("Olive"), QColor("#97ba00")}, @@ -136,28 +136,30 @@ QString Style::getThemeFolder() } -QMap Style::aliasColors = {{TransferGood, "transferGood"}, - {TransferWait, "transferWait"}, - {TransferBad, "transferBad"}, - {TransferMiddle, "transferMiddle"}, - {MainText,"mainText"}, - {NameActive, "nameActive"}, - {StatusActive,"statusActive"}, - {GroundExtra, "groundExtra"}, - {GroundBase, "groundBase"}, - {Orange, "orange"}, - {ThemeDark, "themeDark"}, - {ThemeMediumDark, "themeMediumDark"}, - {ThemeMedium, "themeMedium"}, - {ThemeLight, "themeLight"}, - {Action, "action"}, - {Link, "link"}, - {SearchHighlighted, "searchHighlighted"}, - {SelectText, "selectText"}}; +static const QMap aliasColors = { + {Style::TransferGood, "transferGood"}, + {Style::TransferWait, "transferWait"}, + {Style::TransferBad, "transferBad"}, + {Style::TransferMiddle, "transferMiddle"}, + {Style::MainText,"mainText"}, + {Style::NameActive, "nameActive"}, + {Style::StatusActive,"statusActive"}, + {Style::GroundExtra, "groundExtra"}, + {Style::GroundBase, "groundBase"}, + {Style::Orange, "orange"}, + {Style::ThemeDark, "themeDark"}, + {Style::ThemeMediumDark, "themeMediumDark"}, + {Style::ThemeMedium, "themeMedium"}, + {Style::ThemeLight, "themeLight"}, + {Style::Action, "action"}, + {Style::Link, "link"}, + {Style::SearchHighlighted, "searchHighlighted"}, + {Style::SelectText, "selectText"}, +}; // stylesheet filename, font -> stylesheet // QString implicit sharing deduplicates stylesheets rather than constructing a new one each time -std::map, const QString> Style::stylesheetsCache; +static std::map, const QString> stylesheetsCache; const QString Style::getStylesheet(const QString& filename, const QFont& baseFont) { diff --git a/src/widget/style.h b/src/widget/style.h index 4e92f9ccf..6e06f6727 100644 --- a/src/widget/style.h +++ b/src/widget/style.h @@ -96,11 +96,6 @@ signals: private: Style(); - -private: - static QList themeNameColors; - static std::map, const QString> stylesheetsCache; - static QMap aliasColors; }; #endif // STYLE_H diff --git a/src/widget/widget.cpp b/src/widget/widget.cpp index fbda698bb..f8bf0842e 100644 --- a/src/widget/widget.cpp +++ b/src/widget/widget.cpp @@ -361,7 +361,7 @@ void Widget::init() fileMenu->menu()->addSeparator(); logoutAction = fileMenu->menu()->addAction(QString()); - connect(logoutAction, &QAction::triggered, [this]() { Nexus::getInstance().showLogin(); }); + connect(logoutAction, &QAction::triggered, []() { Nexus::getInstance().showLogin(); }); editMenu = globalMenu->insertMenu(viewMenu, new QMenu(this)); editMenu->menu()->addSeparator(); @@ -1395,7 +1395,6 @@ void Widget::onReceiptReceived(int friendId, ReceiptNum receipt) void Widget::addFriendDialog(const Friend* frnd, ContentDialog* dialog) { - uint32_t friendId = frnd->getId(); const ToxPk& friendPk = frnd->getPublicKey(); ContentDialog* contentDialog = ContentDialogManager::getInstance()->getFriendDialog(friendPk); bool isSeparate = settings.getSeparateWindow(); @@ -1959,8 +1958,7 @@ void Widget::onGroupMessageReceived(int groupnumber, int peernumber, const QStri bool isAction) { const GroupId& groupId = GroupList::id2Key(groupnumber); - Group* g = GroupList::findGroup(groupId); - assert(g); + assert(GroupList::findGroup(groupId)); ToxPk author = core->getGroupPeerPk(groupnumber, peernumber); @@ -2011,8 +2009,7 @@ void Widget::titleChangedByUser(const QString& title) void Widget::onGroupPeerAudioPlaying(int groupnumber, ToxPk peerPk) { const GroupId& groupId = GroupList::id2Key(groupnumber); - Group* g = GroupList::findGroup(groupId); - assert(g); + assert(GroupList::findGroup(groupId)); auto form = groupChatForms[groupId].data(); form->peerAudioPlaying(peerPk); @@ -2313,8 +2310,7 @@ void Widget::setStatusBusy() void Widget::onGroupSendFailed(uint32_t groupnumber) { const auto& groupId = GroupList::id2Key(groupnumber); - Group* g = GroupList::findGroup(groupId); - assert(g); + assert(GroupList::findGroup(groupId)); const auto message = tr("Message failed to send"); const auto curTime = QDateTime::currentDateTime(); diff --git a/test/core/core_test.cpp b/test/core/core_test.cpp index f897c934e..f27c3c5f4 100644 --- a/test/core/core_test.cpp +++ b/test/core/core_test.cpp @@ -106,7 +106,7 @@ void TestCore::startup_without_proxy() test_core = Core::makeToxCore(savedata, settings, err); - if(test_core == nullptr) { + if (test_core == nullptr) { QFAIL("ToxCore initialisation failed"); } @@ -132,7 +132,7 @@ void TestCore::startup_with_invalid_proxy() test_core = Core::makeToxCore(savedata, settings, err); - if(test_core != nullptr) { + if (test_core != nullptr) { QFAIL("ToxCore initialisation passed with invalid SOCKS5 proxy address"); } @@ -144,7 +144,7 @@ void TestCore::startup_with_invalid_proxy() test_core = Core::makeToxCore(savedata, settings, err); - if(test_core != nullptr) { + if (test_core != nullptr) { QFAIL("ToxCore initialisation passed with invalid HTTP proxy address"); } } diff --git a/test/model/messageprocessor_test.cpp b/test/model/messageprocessor_test.cpp index 439dda4b6..b6438185d 100644 --- a/test/model/messageprocessor_test.cpp +++ b/test/model/messageprocessor_test.cpp @@ -39,7 +39,7 @@ class TestMessageProcessor : public QObject Q_OBJECT public: - TestMessageProcessor(){}; + TestMessageProcessor(){} private slots: void testSelfMention(); diff --git a/test/model/sessionchatlog_test.cpp b/test/model/sessionchatlog_test.cpp index b5e7976dd..a1af35530 100644 --- a/test/model/sessionchatlog_test.cpp +++ b/test/model/sessionchatlog_test.cpp @@ -61,7 +61,7 @@ class TestSessionChatLog : public QObject Q_OBJECT public: - TestSessionChatLog(){}; + TestSessionChatLog(){} private slots: void init(); diff --git a/test/persistence/offlinemsgengine_test.cpp b/test/persistence/offlinemsgengine_test.cpp index 5a421d07c..cf4e634d3 100644 --- a/test/persistence/offlinemsgengine_test.cpp +++ b/test/persistence/offlinemsgengine_test.cpp @@ -29,7 +29,7 @@ struct MockFriendMessageSender : public QObject, public ICoreFriendMessageSender Q_OBJECT public: MockFriendMessageSender(Friend* f) - : f(f){}; + : f(f){} bool sendAction(uint32_t friendId, const QString& action, ReceiptNum& receipt) override { return false;