Page MenuHomePhabricator

Return void instead of bool for functions that cannot fail
ClosedPublic

Authored by fpelliccioni on Sep 30 2019, 15:08.

Details

Summary

Return void instead of bool for functions that cannot fail:

  • CBlockTreeDB::ReadReindexing(...)
  • CChainState::ResetBlockFailureFlags(...)
  • CTxMemPool::addUnchecked(...)
  • CWallet::CommitTransaction(...)
  • CWallet::LoadDestData(...)
  • CWallet::LoadKeyMetadata(...)
  • CWallet::LoadScriptMetadata(...)
  • CWallet::LoadToWallet(...)
  • CWallet::SetHDChain(...)
  • CWallet::SetHDSeed(...)
  • PendingWalletTx::commit(...)
  • RemoveLocal(...)
  • SetMinVersion(...)
  • StartHTTPServer(...)
  • StartRPC(...)
  • TorControlConnection::Disconnect(...)

Backport of Bitcoin Core PR13774
https://github.com/bitcoin/bitcoin/pull/13774

Test Plan
make check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
feature-backport-ad51e1372
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 7666
Build 13371: Bitcoin ABC Buildbot (legacy)
Build 13370: arc lint + arc unit

Event Timeline

jasonbcox requested changes to this revision.Sep 30 2019, 16:33
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/validation.cpp
3239 ↗(On Diff #13274)

This function can return false (and see build failure).

3264 ↗(On Diff #13274)

ditto

This revision now requires changes to proceed.Sep 30 2019, 16:33

I see that you use make check-all in many tests. If you are using cmake, you should use ninja as it is faster. But right now, cmake is only a secondary option, so make sure the autotool build works.

src/validation.cpp
3239 ↗(On Diff #13274)

The function you have the comment on, yes. Hopefully, this isn't the function that is being changed.

The test plan is meant to actually be ran.

This revision is now accepted and ready to land.Oct 3 2019, 00:24

I see that you use make check-all in many tests. If you are using cmake, you should use ninja as it is faster. But right now, cmake is only a secondary option, so make sure the autotool build works.

I usually use both ways to compile.
Unfortunately on my computer, ninja is not much faster than make.

The test plan is meant to actually be ran.

Yes, I always run the test plan.
I really don't know what happened to me here.
Sorry.