wallet: Introduce assertion to document the assumption that cache and cache_used are always set in tandem #13683

1. 
   practicalswift commented at 8:25 AM on July 17, 2018: contributor

   Avoid potential null pointer dereference in CWalletTx::GetAvailableCredit(...).

   Introduced in 4279da47855ec776f8d57c6579fe89afc9cbe8c1.

2. fanquake added the label Wallet on Jul 17, 2018
3. wallet: Avoid potential null pointer dereference in CWalletTx::GetAvailableCredit(...) d06330396f
4. in src/wallet/wallet.cpp:1950 in 3256f9c849 outdated

   1946 | @@ -1947,7 +1947,9 @@ CAmount CWalletTx::GetAvailableCredit(bool fUseCache, const isminefilter& filter
   1947 |  
   1948 |      if (cache) {
   1949 |          *cache = nCredit;
   1950 | -        *cache_used = true;
   1951 | +        if (cache_used) {
   

   ---

   ---

   

   promag commented at 8:56 AM on July 17, 2018:

   If cache is set then cache_used is also set. Maybe assert(cache_used) instead?

5. practicalswift force-pushed on Jul 17, 2018
6. 
   practicalswift commented at 8:58 AM on July 17, 2018: contributor

   @promag Please re-review :-)

7. 
   promag commented at 9:02 AM on July 17, 2018: member

   utACK d063303.

8. fanquake requested review from jnewbery on Jul 18, 2018
9. 
   laanwj commented at 2:44 PM on July 18, 2018: member

   So is an assert acceptable here? When can this happen? Is it acceptable for the process to crash when it happens, or would this need actual error handling?

10. 
    MarcoFalke commented at 2:48 PM on July 18, 2018: member

    utACK d06330396f64b9a3a3016afc1f937633b4b322ab. They are always set in tandem. Though, there is no "potential null pointer dereference", please adjust the title.

11. 
    laanwj commented at 3:49 PM on July 18, 2018: member

    Indeed

12. practicalswift renamed this:
    wallet: Avoid potential null pointer dereference in CWalletTx::GetAvailableCredit(...)
    wallet: Introduce assertion to document the assumption that cache and cache_used are always set in tandem
    on Jul 18, 2018
13. 
    practicalswift commented at 6:20 PM on July 18, 2018: contributor

    @MarcoFalke Thanks for the review and the utACK. Title now updated :-)

14. MarcoFalke merged this on Jul 22, 2018
15. MarcoFalke closed this on Jul 22, 2018

    ---

16. MarcoFalke referenced this in commit e8c74348d3 on Jul 22, 2018
17. PastaPastaPasta referenced this in commit 9d64a15c6d on Feb 2, 2021
18. PastaPastaPasta referenced this in commit 7b8eb59178 on Feb 4, 2021
19. practicalswift deleted the branch on Apr 10, 2021
20. gades referenced this in commit 1e92893afa on Mar 22, 2022
21. DrahtBot locked this on Aug 16, 2022

Contributors

practicalswiftpromaglaanwjMarcoFalke

Review Requested
jnewbery