fuzz: silence a compiler warning about unused CBanEntry comparator #22557

1. 
   vasild commented at 4:03 pm on July 26, 2021: member

   0test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
   1static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
   2            ^
   31 warning generated.
   

   See #22517 (comment)

2. DrahtBot added the label Tests on Jul 26, 2021
3. 
   hebasto commented at 6:03 pm on July 26, 2021: member

   See #22517 (comment)

   Actually, I mentioned it expecting no one will silent a temporary warning :smile:

4. 
   jarolrod commented at 6:33 pm on July 26, 2021: member

   No strong opinion on this. It will be interesting to see the response to this.

   Do we want to have a policy of silencing temporary warnings when their is a clear plan that will silence them? — this warning will be silenced when the fix for the banman fuzz test is implemented.

   On the other hand, the argument could be made that #22517 should not have allowed this warning to come up and this PR can be seen as a follow-up to that PR. A followup which finishes properly temporarily disabling the assert.

   Tested that this silences the warning on Ubuntu 20.04: master:

   0$ make 2>&1 > /dev/null | grep -o "Wunused-function" | wc -l
   1
   21
   

   pr:

   0$ make 2>&1 > /dev/null | grep -o "Wunused-function" | wc -l
   1
   20
   

5. 
   vasild commented at 7:09 am on July 27, 2021: member

   This warning disturbs developers’ workflow (compilation with -Werror fails).

   It does not matter whether it is temporary, semi-temporary, permanently-temporary, temo-low or should just be painted in purple color.

6. 
   MarcoFalke commented at 8:23 am on July 27, 2021: member

   cr ACK 83dd35260b30eaedc8822326f75c037d5e51f883

   Sorry didn’t see the warning in my patch

7. fuzz: silence a compiler warning about unused CBanEntry comparator

   ```
   test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function]
   static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs)
               ^
   1 warning generated.
   ```

   787296eb67
8. in src/test/fuzz/banman.cpp:119 in 83dd35260b outdated

   114@@ -109,8 +115,9 @@ FUZZ_TARGET_INIT(banman, initialize_banman)
   115             BanMan ban_man_read{banlist_file, /* client_interface */ nullptr, /* default_ban_time */ 0};
   116             banmap_t banmap_read;
   117             ban_man_read.GetBanned(banmap_read);
   118-            // Temporarily disabled to allow the remainder of the fuzz test to run while a fix is being worked on:
   119-            // assert(banmap == banmap_read);
   120+#ifdef ENABLE_BANMAN_COMPARE
   121+            assert(banmap == banmap_read);
   

   ---

   ---

   

   MarcoFalke commented at 9:23 am on July 27, 2021:

   nit: wouldn’t it be easier to just:

   0            (void)(banmap == banmap_read);
   

   ---

   

   vasild commented at 12:00 pm on July 27, 2021:

   Yes, that is shorter, thanks!
9. vasild force-pushed on Jul 27, 2021
10. 
    vasild commented at 11:59 am on July 27, 2021: member

    83dd35260b...787296eb67: take review suggestion
11. hebasto approved
12. 
    hebasto commented at 12:05 pm on July 27, 2021: member

    ACK 787296eb6744b15ab693c053e4030ff68dfc95e0
13. 
    laanwj commented at 12:55 pm on July 27, 2021: member

    I don’t think we need to follow every temporary change with a change to clean up warnings. If there is intent to use this function soon, I do not see the point of merging an intermediate PR that removes it just to be reintroduced.

    Edit: oh, you didn’t remove it. This seems fine to me (though still I don’t think we should be too busybody about this).

14. 
    MarcoFalke commented at 3:50 pm on July 27, 2021: member

    cr ACK 787296eb6744b15ab693c053e4030ff68dfc95e0

    again my fault for not noticing the compiler warning in the initial patch

15. 
    practicalswift commented at 3:59 pm on July 27, 2021: contributor

    cr ACK 787296eb6744b15ab693c053e4030ff68dfc95e0
16. fanquake merged this on Jul 28, 2021
17. fanquake closed this on Jul 28, 2021

    ---

18. vasild deleted the branch on Jul 28, 2021
19. sidhujag referenced this in commit 28706338a4 on Jul 28, 2021
20. hebasto referenced this in commit 9214541e00 on Aug 6, 2021
21. 
    hebasto commented at 3:54 pm on August 6, 2021: member

    ~Backported in #22629.~

    UPDATE: #22629 (comment)

22. hebasto referenced this in commit 482ab6d3bb on Aug 6, 2021
23. hebasto referenced this in commit 20c1331b84 on Aug 7, 2021
24. hebasto referenced this in commit c427a01e01 on Aug 9, 2021
25. hebasto referenced this in commit b5968dff37 on Aug 9, 2021
26. hebasto referenced this in commit 7e58f5cf1a on Aug 19, 2021
27. hebasto referenced this in commit e3d207d6ee on Aug 20, 2021
28. hebasto referenced this in commit f6c4c79705 on Aug 20, 2021
29. DrahtBot locked this on Aug 20, 2022

Contributors
vasild hebasto jarolrod MarcoFalke laanwj practicalswift

Labels
Tests