Page MenuHomePhabricator
Differential D4179

Return void instead of bool for functions that cannot fail
ClosedPublic
Actions

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

Details

Reviewers
Fabien
deadalnix
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGINGa0dd249c1862: Return void instead of bool for functions that cannot fail
rABCa0dd249c1862: Return void instead of bool for functions that cannot fail
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

fpelliccioni created this revision.Sep 30 2019, 15:08
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 30 2019, 15:08
Herald added a reviewer: deadalnix. · View Herald Transcript
teamcity added a comment.Sep 30 2019, 15:08

Build Bitcoin-ABC / Diffs / Diff Testing started.

teamcity added a comment.Sep 30 2019, 15:10

Build Bitcoin-ABC / Diffs / Diff Testing failed.

Harbormaster failed remote builds in B7634: Diff 13274!Sep 30 2019, 15:10
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
fpelliccioni updated this revision to Diff 13320.Oct 2 2019, 14:15

fixes building errors.

teamcity added a comment.Oct 2 2019, 14:15

Build Bitcoin-ABC / Diffs / Diff Testing started.

Harbormaster completed remote builds in B7666: Diff 13320.Oct 2 2019, 14:32
teamcity added a comment.Oct 2 2019, 14:32

Build Bitcoin-ABC / Diffs / Diff Testing passed.

deadalnix added a comment.Oct 2 2019, 20:58

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.

deadalnix added inline comments.Oct 2 2019, 21:00
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.

deadalnix accepted this revision.Oct 2 2019, 21:08

The test plan is meant to actually be ran.

jasonbcox accepted this revision.Oct 3 2019, 00:24
This revision is now accepted and ready to land.Oct 3 2019, 00:24
fpelliccioni added a comment.Oct 3 2019, 13:31
In D4179#100728, @deadalnix wrote:

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.

fpelliccioni added a comment.Oct 3 2019, 13:32
In D4179#100740, @deadalnix wrote:

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.

Closed by commit rABCa0dd249c1862: Return void instead of bool for functions that cannot fail (authored by practicalswift <practicalswift@users.noreply.github.com>, committed by fpelliccioni). · Explain WhyOct 3 2019, 13:33
This revision was automatically updated to reflect the committed changes.
fpelliccioni added a commit: rABCa0dd249c1862: Return void instead of bool for functions that cannot fail.
fpelliccioni added a commit: rSTAGINGa0dd249c1862: Return void instead of bool for functions that cannot fail.Oct 9 2019, 20:10
nakihito mentioned this in D4304: Merge #9662: Add createwallet "disableprivatekeys" option: a sane mode for watchonly-wallets.Nov 19 2019, 19:28

Revision Contents
Changeset List

PathSize
src/
2 lines
3 lines
11 lines
rpc/
2 lines
3 lines
5 lines
2 lines
3 lines
4 lines
6 lines
6 lines
18 lines
wallet/
14 lines
35 lines
14 lines

Diff 13320

View Options

src/httpserver.h

Loading...
View Options

src/httpserver.cpp

Loading...
View Options

src/init.cpp

Loading...
View Options

src/rpc/server.h

Loading...
View Options

src/rpc/server.cpp

Loading...
View Options

src/torcontrol.cpp

Loading...
View Options

src/txdb.h

Loading...
View Options

src/txdb.cpp

Loading...
View Options

src/txmempool.h

Loading...
View Options

src/txmempool.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...
View Options

src/wallet/wallet.h

Loading...
View Options

src/wallet/wallet.cpp

Loading...
View Options

src/wallet/walletdb.cpp

Loading...