wallet: Set encrypted_batch to nullptr after delete. Avoid double free in the case of NDEBUG. #14138

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:set-to-nullptr-after-delete changing 1 files +2 −0

practicalswift commented at 1:43 PM on September 3, 2018: contributor

Set encrypted_batch to nullptr after delete. Avoid double free in the case of NDEBUG.
wallet: Set encrypted_batch to nullptr after delete. Avoid double free in the case of NDEBUG. fa462b3657
fanquake added the label Wallet on Sep 3, 2018
domob1812 commented at 3:51 PM on September 3, 2018: contributor

utACK fa462b365715928d2c6eb52ca6a448f367084f0f.

I wonder, though: Are we "explicitly" supporting running a binary compiled with NDEBUG? If so, then we should probably not use assert(false) in the two places here (and instead explicitly print an error message and exit using other means).
gmaxwell commented at 4:03 PM on September 3, 2018: contributor

@domob1812 We explicitly do not support that, but at the same time the code should not contain strange bugs when run that way. This is not an argument against assert(false)-- that should be used to protect invariants where if violated potentially things would happen elsewhere.
domob1812 commented at 4:06 PM on September 3, 2018: contributor

@gmaxwell: Thanks for the clarification, that makes sense. I agree that we should still fix bugs like this.
donaloconnor commented at 7:52 PM on September 3, 2018: contributor

utACK https://github.com/bitcoin/bitcoin/commit/fa462b365715928d2c6eb52ca6a448f367084f0f
DrahtBot commented at 10:45 AM on September 4, 2018: member
Note to reviewers: This pull request conflicts with the following ones:
- #14144 (Refactoring: Clarify code using encrypted_batch in CWallet by domob1812)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
laanwj commented at 3:59 PM on September 10, 2018: member

utACK fa462b365715928d2c6eb52ca6a448f367084f0f
ken2812221 referenced this in commit 6eeac2e628 on Sep 10, 2018
laanwj merged this on Sep 10, 2018
laanwj closed this on Sep 10, 2018
practicalswift deleted the branch on Apr 10, 2021
PastaPastaPasta referenced this in commit 5d4f73523a on Jun 27, 2021
PastaPastaPasta referenced this in commit a4762be1f0 on Jun 28, 2021
PastaPastaPasta referenced this in commit 37d60ce757 on Jun 29, 2021
PastaPastaPasta referenced this in commit 1d37f75cd1 on Jul 1, 2021
gades referenced this in commit 72060b1c5a on Apr 18, 2022
DrahtBot locked this on Aug 18, 2022

Contributors

Labels

Linked (view graph)

#14139 Clean up encrypted_batch in wallet #14144 Refactoring: Clarify code using encrypted_batch in CWallet