Build and run bitcoin
Details
Details
- Reviewers
freetrader deadalnix - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) Restricted Project
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- master
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 518 Build 518: arc lint + arc unit
Event Timeline
Comment Actions
You don't have to accept this patch .. I basically wanted it here just in case you guys decide to get paranoid and catch every time CNodeState might be null.
I haven't even managed to reproduce the crash in a debugger (it seems to be somewhat intermittent) so I can't be sure this is what's causing it, although I think @freetrader did say that is the reason for the crash.
Comment Actions
First please rebase, there are a lot of changes in that that are not relevant on master.
Second, this is adding a ton of logic. Adding a ton of logic makes the code more complex, therefore for error prone, not less. A lot of boilerplate is a sign of poorly designed API.
Comment Actions
I completely agree that a lot of boilerplate is a sign of a bad API. Totally agreed. Like I said, this is just here for reference in case someone gets desperate about the segfault reported.
I agree that it touches a lot of logic, and might introduce new bugs (even though I was careful).
I haven't been able to reproduce the bug reliably anyway. I've been running this changed version for a few hours now and not 1 error message related to nodestate being null..
If you want I can close this or delete it.
I put it up here just for reference in case someone wanted to troubleshoot the bug and have an alternate codepath that is paranoid about that null pointer.
Safe to ignore this..
Comment Actions
Useless revision. Was here for my testing. Sorry if this polluted phabricator. Still getting used to phabricator workflow.