From e95c6e085355045c5b11a4e800f4a42f6e43b768 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 9 Jul 2019 11:47:55 +0200 Subject: [PATCH] Merge #16348: qt: Assert QMetaObject::invokeMethod result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 64fee489448c62319e77941c30152084695b5a5d qt: Assert QMetaObject::invokeMethod result (João Barbosa) f27bd96b5fdc2921d93c44bbf422bff0e979c4de gui: Fix missing qRegisterMetaType(WalletModel*) (João Barbosa) Pull request description: Invalid/wrong dynamic calls aren't verified by the compiler. This PR asserts those dynamic calls. Once we bump Qt to at least 5.10 these can be refactored to use the `invokeMethod` overload that allows connecting to lambdas or member pointers, which are compile checked. For reference, one of the overloaded versions is https://doc.qt.io/qt-5/qmetaobject.html#invokeMethod-5. ACKs for top commit: laanwj: ACK 64fee489448c62319e77941c30152084695b5a5d Tree-SHA512: d332e5d7eb2c7be5d3fe90e2e4ff20a67800b9664f6637c122a23647a964f7915703d3f086e2de440f695cfe14de268ff581d0092b7736e911952a4f4d248e25 --- src/qt/bitcoingui.cpp | 3 ++- src/qt/clientmodel.cpp | 24 ++++++++++++++++-------- src/qt/dash.cpp | 3 +++ src/qt/splashscreen.cpp | 3 ++- src/qt/test/wallettests.cpp | 3 ++- src/qt/transactiontablemodel.cpp | 18 ++++++++++++------ src/qt/walletcontroller.cpp | 3 ++- src/qt/walletmodel.cpp | 27 ++++++++++++++++++--------- 8 files changed, 57 insertions(+), 27 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index d3810e22de..8e2c4c6426 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1856,12 +1856,13 @@ static bool ThreadSafeMessageBox(BitcoinGUI* gui, const std::string& message, co style &= ~CClientUIInterface::SECURE; bool ret = false; // In case of modal message, use blocking connection to wait for user to click a button - QMetaObject::invokeMethod(gui, "message", + bool invoked = QMetaObject::invokeMethod(gui, "message", modal ? GUIUtil::blockingGUIThreadConnection() : Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(caption)), Q_ARG(QString, QString::fromStdString(message)), Q_ARG(unsigned int, style), Q_ARG(bool*, &ret)); + assert(invoked); return ret; } diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index ba10ac6e43..f7b36b172e 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -217,34 +217,39 @@ void ClientModel::updateBanlist() static void ShowProgress(ClientModel *clientmodel, const std::string &title, int nProgress) { // emits signal "showProgress" - QMetaObject::invokeMethod(clientmodel, "showProgress", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(clientmodel, "showProgress", Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(title)), Q_ARG(int, nProgress)); + assert(invoked); } static void NotifyNumConnectionsChanged(ClientModel *clientmodel, int newNumConnections) { // Too noisy: qDebug() << "NotifyNumConnectionsChanged: " + QString::number(newNumConnections); - QMetaObject::invokeMethod(clientmodel, "updateNumConnections", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(clientmodel, "updateNumConnections", Qt::QueuedConnection, Q_ARG(int, newNumConnections)); + assert(invoked); } static void NotifyNetworkActiveChanged(ClientModel *clientmodel, bool networkActive) { - QMetaObject::invokeMethod(clientmodel, "updateNetworkActive", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(clientmodel, "updateNetworkActive", Qt::QueuedConnection, Q_ARG(bool, networkActive)); + assert(invoked); } static void NotifyAlertChanged(ClientModel *clientmodel) { qDebug() << "NotifyAlertChanged"; - QMetaObject::invokeMethod(clientmodel, "updateAlert", Qt::QueuedConnection); + bool invoked = QMetaObject::invokeMethod(clientmodel, "updateAlert", Qt::QueuedConnection); + assert(invoked); } static void BannedListChanged(ClientModel *clientmodel) { qDebug() << QString("%1: Requesting update for peer banlist").arg(__func__); - QMetaObject::invokeMethod(clientmodel, "updateBanlist", Qt::QueuedConnection); + bool invoked = QMetaObject::invokeMethod(clientmodel, "updateBanlist", Qt::QueuedConnection); + assert(invoked); } static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int height, int64_t blockTime, const std::string& strBlockHash, double verificationProgress, bool fHeader) @@ -267,12 +272,13 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int heig // During initial sync, block notifications, and header notifications from reindexing are both throttled. if (!initialSync || (fHeader && !clientmodel->node().getReindex()) || now - nLastUpdateNotification > MODEL_UPDATE_DELAY) { //pass an async signal to the UI thread - QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection, Q_ARG(int, height), Q_ARG(QDateTime, QDateTime::fromTime_t(blockTime)), Q_ARG(QString, QString::fromStdString(strBlockHash)), Q_ARG(double, verificationProgress), Q_ARG(bool, fHeader)); + assert(invoked); nLastUpdateNotification = now; } } @@ -280,9 +286,10 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int heig static void NotifyChainLock(ClientModel *clientmodel, const std::string& bestChainLockHash, int bestChainLockHeight) { // emits signal "chainlockChanged" - QMetaObject::invokeMethod(clientmodel, "chainLockChanged", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(clientmodel, "chainLockChanged", Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(bestChainLockHash)), Q_ARG(int, bestChainLockHeight)); + assert(invoked); } static void NotifyMasternodeListChanged(ClientModel *clientmodel, const CDeterministicMNList& newList) @@ -292,8 +299,9 @@ static void NotifyMasternodeListChanged(ClientModel *clientmodel, const CDetermi static void NotifyAdditionalDataSyncProgressChanged(ClientModel *clientmodel, double nSyncProgress) { - QMetaObject::invokeMethod(clientmodel, "additionalDataSyncProgressChanged", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(clientmodel, "additionalDataSyncProgressChanged", Qt::QueuedConnection, Q_ARG(double, nSyncProgress)); + assert(invoked); } void ClientModel::subscribeToCoreSignals() diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp index 79c426c43c..22d36290cb 100644 --- a/src/qt/dash.cpp +++ b/src/qt/dash.cpp @@ -473,6 +473,9 @@ int GuiMain(int argc, char* argv[]) // Register meta types used for QMetaObject::invokeMethod qRegisterMetaType< bool* >(); +#ifdef ENABLE_WALLET + qRegisterMetaType(); +#endif // Need to pass name here as CAmount is a typedef (see http://qt-project.org/doc/qt-5/qmetatype.html#qRegisterMetaType) // IMPORTANT if it is no longer a typedef use the normal variant above qRegisterMetaType< CAmount >("CAmount"); diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 7d1032ccd4..64610f88d4 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -161,11 +161,12 @@ void SplashScreen::finish() static void InitMessage(SplashScreen *splash, const std::string &message) { - QMetaObject::invokeMethod(splash, "showMessage", + bool invoked = QMetaObject::invokeMethod(splash, "showMessage", Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(message)), Q_ARG(int, Qt::AlignBottom | Qt::AlignHCenter), Q_ARG(QColor, GUIUtil::getThemedQColor(GUIUtil::ThemedColor::DEFAULT))); + assert(invoked); } static void ShowProgress(SplashScreen *splash, const std::string &title, int nProgress, bool resume_possible) diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index ca59c44bc5..32a2678184 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -67,7 +67,8 @@ uint256 SendCoins(CWallet& wallet, SendCoinsDialog& sendCoinsDialog, const CTxDe if (status == CT_NEW) txid = hash; })); ConfirmSend(); - QMetaObject::invokeMethod(&sendCoinsDialog, "on_sendButton_clicked"); + bool invoked = QMetaObject::invokeMethod(&sendCoinsDialog, "on_sendButton_clicked"); + assert(invoked); return txid; } diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index d935ee26a6..c3d07c761a 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -753,10 +753,11 @@ public: { QString strHash = QString::fromStdString(hash.GetHex()); qDebug() << "NotifyTransactionChanged: " + strHash + " status= " + QString::number(status); - QMetaObject::invokeMethod(ttm, "updateTransaction", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(ttm, "updateTransaction", Qt::QueuedConnection, Q_ARG(QString, strHash), Q_ARG(int, status), Q_ARG(bool, showTransaction)); + assert(invoked); } private: uint256 hash; @@ -785,12 +786,13 @@ static void NotifyTransactionChanged(TransactionTableModel *ttm, const uint256 & static void NotifyAddressBookChanged(TransactionTableModel *ttm, const CTxDestination &address, const std::string &label, bool isMine, const std::string &purpose, ChangeType status) { - QMetaObject::invokeMethod(ttm, "updateAddressBook", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(ttm, "updateAddressBook", Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(EncodeDestination(address))), Q_ARG(QString, QString::fromStdString(label)), Q_ARG(bool, isMine), Q_ARG(QString, QString::fromStdString(purpose)), Q_ARG(int, (int)status)); + assert(invoked); } static void ShowProgress(TransactionTableModel *ttm, const std::string &title, int nProgress) @@ -801,12 +803,16 @@ static void ShowProgress(TransactionTableModel *ttm, const std::string &title, i if (nProgress == 100) { fQueueNotifications = false; - if (vQueueNotifications.size() > 10) // prevent balloon spam, show maximum 10 balloons - QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); + if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons + bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); + assert(invoked); + } for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) { - if (vQueueNotifications.size() - i <= 10) - QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); + if (vQueueNotifications.size() - i <= 10) { + bool invoked = QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); + assert(invoked); + } vQueueNotifications[i].invoke(ttm); } diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 0c7509973b..92d66eb996 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -67,7 +67,8 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptrmoveToThread(thread()); - QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, Q_ARG(WalletModel*, wallet_model)); + bool invoked = QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, Q_ARG(WalletModel*, wallet_model)); + assert(invoked); } return wallet_model; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 7465d0d9df..7c1f10745f 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -440,13 +440,15 @@ int64_t WalletModel::getKeysLeftSinceAutoBackup() const static void NotifyUnload(WalletModel* walletModel) { qDebug() << "NotifyUnload"; - QMetaObject::invokeMethod(walletModel, "unload"); + bool invoked = QMetaObject::invokeMethod(walletModel, "unload"); + assert(invoked); } static void NotifyKeyStoreStatusChanged(WalletModel *walletmodel) { qDebug() << "NotifyKeyStoreStatusChanged"; - QMetaObject::invokeMethod(walletmodel, "updateStatus", Qt::QueuedConnection); + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateStatus", Qt::QueuedConnection); + assert(invoked); } static void NotifyAddressBookChanged(WalletModel *walletmodel, @@ -458,49 +460,56 @@ static void NotifyAddressBookChanged(WalletModel *walletmodel, QString strPurpose = QString::fromStdString(purpose); qDebug() << "NotifyAddressBookChanged: " + strAddress + " " + strLabel + " isMine=" + QString::number(isMine) + " purpose=" + strPurpose + " status=" + QString::number(status); - QMetaObject::invokeMethod(walletmodel, "updateAddressBook", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateAddressBook", Qt::QueuedConnection, Q_ARG(QString, strAddress), Q_ARG(QString, strLabel), Q_ARG(bool, isMine), Q_ARG(QString, strPurpose), Q_ARG(int, status)); + assert(invoked); } static void NotifyTransactionChanged(WalletModel *walletmodel, const uint256 &hash, ChangeType status) { Q_UNUSED(hash); Q_UNUSED(status); - QMetaObject::invokeMethod(walletmodel, "updateTransaction", Qt::QueuedConnection); + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateTransaction", Qt::QueuedConnection); + assert(invoked); } static void NotifyISLockReceived(WalletModel *walletmodel) { - QMetaObject::invokeMethod(walletmodel, "updateNumISLocks", Qt::QueuedConnection); + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateNumISLocks", Qt::QueuedConnection); + assert(invoked); } static void NotifyChainLockReceived(WalletModel *walletmodel, int chainLockHeight) { - QMetaObject::invokeMethod(walletmodel, "updateChainLockHeight", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateChainLockHeight", Qt::QueuedConnection, Q_ARG(int, chainLockHeight)); + assert(invoked); } static void ShowProgress(WalletModel *walletmodel, const std::string &title, int nProgress) { // emits signal "showProgress" - QMetaObject::invokeMethod(walletmodel, "showProgress", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(walletmodel, "showProgress", Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(title)), Q_ARG(int, nProgress)); + assert(invoked); } static void NotifyWatchonlyChanged(WalletModel *walletmodel, bool fHaveWatchonly) { - QMetaObject::invokeMethod(walletmodel, "updateWatchOnlyFlag", Qt::QueuedConnection, + bool invoked = QMetaObject::invokeMethod(walletmodel, "updateWatchOnlyFlag", Qt::QueuedConnection, Q_ARG(bool, fHaveWatchonly)); + assert(invoked); } static void NotifyCanGetAddressesChanged(WalletModel* walletmodel) { - QMetaObject::invokeMethod(walletmodel, "canGetAddressesChanged"); + bool invoked = QMetaObject::invokeMethod(walletmodel, "canGetAddressesChanged"); + assert(invoked); } void WalletModel::subscribeToCoreSignals()