Bitcoin-Qt: tweak Qt walletXXX.cpp/h code#2872
Bitcoin-Qt: tweak Qt walletXXX.cpp/h code#2872laanwj merged 1 commit intobitcoin:masterfrom Diapolo:GUI_wallet
Conversation
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f6603646f33c3664c3cb3e5faac253b25450a9b1 for binaries and test log. |
|
@laanwj Can you take a look? |
src/qt/walletview.cpp
Outdated
There was a problem hiding this comment.
Don't remove the NULL pointer check here. It is valid to do a setWalletModel(NULL) (for example, as part of a shutdown sequence).
There was a problem hiding this comment.
What about the change above in setClientModel() should we define as coding-style default that all setModel() functions should include the NULL pointer check?
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/33b1328909711d51ef1b8820fcf676256ad21776 for binaries and test log. |
src/qt/walletview.cpp
Outdated
There was a problem hiding this comment.
Why remove the null pointer checks here?
There was a problem hiding this comment.
That slot is connected in WalletView::setWalletModel(), which covers the walletModel check and WalletView::setClientModel() covers the clientModel check.
There was a problem hiding this comment.
You're correct. Although I don't think the slot gets disconnected when a NULL model would be set later on, so it's not completely impossible to reach here with model being NULL.
If there's anything I've learned in years of writing C++ code it's that there is no such thing as too many NULL pointer checks. Better to be safe than sorry, please just keep it :)
There was a problem hiding this comment.
I'm fine with re-adding them ;-). I guess such a check is also not an expensive operation, right?
There was a problem hiding this comment.
Indeed, it's not, just a 32/64 bit compare, processors can do billions of those per second. It's also only called once per incoming transaction.
|
Updated to revert the removal of NULL pointer checks to comply with @laanwj :). |
WalletView: - add new signal showNormalIfMinimized() - emit the new signal in handleURI() to fix a bug, preventing the main window to show up when using bitcoin: URIs WalletStack: - connect the showNormalIfMinimized() signal from WalletView with the showNormalIfMinimized() slot in BitcoinGUI - rework setCurrentWallet() to return a bool - add check for valid walletModel in addWallet() - add missing gui attribute initialisation in constructor WalletFrame: - remove unused or unneded class attributes gui and clientModel - add a check for valid clientModel in setClientModel() General: - small code formatting changes
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/dbc0a6aba2cf94aa1b167145a18e0b9c671aef5b for binaries and test log. |
Bitcoin-Qt: tweak Qt walletXXX.cpp/h code
This allows AlreadyHave to check if an announced (via INV) islock was already known in the past. This avoids requesting islocks which got obsolete due to ChainLocks.
5fa28e9 refactor: Remove unused signal (Hennadii Stepanov) Pull request description: `WalletView::showNormalIfMinimized()` signal was introduced in #2872 (dbc0a6a). The only signal emit command was removed in #3144 (2384a28) ACKs for top commit: promag: ACK 5fa28e9. practicalswift: ACK 5fa28e9: nice find emilengler: ACK 5fa28e9 jonasschnelli: utACK 5fa28e9 Tree-SHA512: 4714acf8c683594d3c00523c7b14bc6b94d469418f0cebe4f4b5266ca0e4c45c80d4caf358739eae9231ee4a69c9c902caeb35f3866b99443cf653f89d6d825b
4 participants
WalletView:
window to show up when using bitcoin: URIs
WalletStack:
showNormalIfMinimized() slot in BitcoinGUI
WalletFrame:
General: