refactor: clean up -Wlogical-op warning in wallet/scriptpubkeyman.cpp #19986

1. 
   maskoficarus commented at 8:11 pm on September 20, 2020: none

   This is a quick patch that fixes #19912 . This change prevents a -Wlogical-op warning that occurs because we’re treating a const int value as a boolean. There’s no sense checking if a non-zero constant has a value, so I’ve removed the check.

   #18836 also addresses the same warning, but has a larger scope and will require more review. This pull request will act as a patch to prevent this compile warning until 18836 is merged.

2. maskoficarus renamed this:
   Update scriptpubkeyman.cpp
   refactor: clean up -Wlogical-op warning in wallet/scriptpubkeyman.cp
   on Sep 20, 2020
3. 
   hebasto commented at 9:08 pm on September 20, 2020: member

   Thanks @maskoficarus !

   #19912 (comment), #19912 (comment)

4. DrahtBot added the label Refactoring on Sep 20, 2020
5. DrahtBot added the label Wallet on Sep 20, 2020
6. 
   kristapsk commented at 9:36 pm on September 20, 2020: contributor

   I’m not sure I get this. VERSION_HD_CHAIN_SPLIT is a constant, so what’s the point of checking it’s value here at all?
7. 
   maskoficarus commented at 0:11 am on September 21, 2020: none

   I’m not sure I get this. VERSION_HD_CHAIN_SPLIT is a constant, so what’s the point of checking it’s value here at all?

   Good point. It shouldn’t be needed.

8. 
   DrahtBot commented at 4:57 am on September 21, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #18836 (wallet: upgradewallet fixes and additional tests by achow101)

   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.

9. 
   MarcoFalke commented at 8:03 pm on September 22, 2020: member

   Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
10. 
    fanquake commented at 12:34 pm on September 29, 2020: member

    Thanks for squashing, however if we’re going to merge this you’ll need to use a commit message that is more useful/explanatory than the GitHub default. See CONTRIBUTING.md#committing-patches.
11. refactor: Clean up -Wlogical-op warning

    This commit fixes #19912 by removing a check that always returned true. That check was causing a -Wlogical-op warning because it treated a constant int as though it were a boolean.

    95fedd33a2
12. 
    practicalswift commented at 8:36 am on October 9, 2020: contributor

    @achow101 As the author of 415afcccd3e5583defdb76e3a280f48e98983301, would you mind reviewing this to make sure the the logic @maskoficarus suggests corresponds to what was originally intended? :)
13. 
    MarcoFalke commented at 8:49 am on October 9, 2020: member

    review ACK 95fedd33a23d6cb7542378afef229965f1ebfd68

    Though, would be good to have instructions on how to reproduce the warning

14. 
    kristapsk commented at 10:35 am on October 9, 2020: contributor

    would be good to have instructions on how to reproduce the warning

    I was able to trigger this warning just by building with GCC 9.2.0 (on Gentoo Linux).

15. 
    practicalswift commented at 1:48 pm on October 9, 2020: contributor

    @MarcoFalke gcc 9.3 which is the default compiler in Ubuntu 20.04 prints this diagnostic by default :)
16. hebasto approved
17. 
    hebasto commented at 3:07 pm on October 18, 2020: member

    ACK 95fedd33a23d6cb7542378afef229965f1ebfd68, tested on Linux Mint 20 (x86_64):

    • master:

     0$ gcc --version
     1gcc (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
     2Copyright (C) 2019 Free Software Foundation, Inc.
     3This is free software; see the source for copying conditions.  There is NO
     4warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
     5
     6$ git rev-parse HEAD
     7b99a1633b270e0e89479b2bb2ae19a8a8dc0fa05
     8$ make > /dev/null 
     9wallet/scriptpubkeyman.cpp: In member function ‘virtual bool LegacyScriptPubKeyMan::Upgrade(int, bilingual_str&)’:
    10wallet/scriptpubkeyman.cpp:455:55: warning: logical ‘and’ applied to non-boolean constant [-Wlogical-op]
    11  455 |     if (m_storage.CanSupportFeature(FEATURE_HD_SPLIT) && CHDChain::VERSION_HD_CHAIN_SPLIT) {
    12      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    

    • PR:

    0$ git rev-parse HEAD
    195fedd33a23d6cb7542378afef229965f1ebfd68
    2$ make > /dev/null 
    3# no output :)
    

18. fanquake renamed this:
    refactor: clean up -Wlogical-op warning in wallet/scriptpubkeyman.cp
    refactor: clean up -Wlogical-op warning in wallet/scriptpubkeyman.cpp
    on Oct 19, 2020
19. fanquake merged this on Oct 19, 2020
20. fanquake closed this on Oct 19, 2020

    ---

21. sidhujag referenced this in commit cfc58deb8b on Oct 19, 2020
22. MarcoFalke referenced this in commit e2ae6a2bef on Dec 4, 2020
23. Fabcien referenced this in commit a46f4c08b3 on Nov 25, 2021
24. DrahtBot locked this on Feb 15, 2022

Contributors
maskoficarus hebasto kristapsk DrahtBot MarcoFalke fanquake practicalswift

Labels
Refactoring Wallet