Fix -Wunreachable-code-loop-increment warning #19017

1. 
   fanquake commented at 2:33 pm on May 19, 2020: member

   In #19015 it’s been suggested that we add some new compiler warnings to our build. Some of these, such as -Wunreachable-code-loop-increment, generate warnings. We’ll likely want to fix these up if we’re going to turn these warnings on.

   0init.cpp:969:5: warning: loop will run at most once (loop increment never executed) [-Wunreachable-code-loop-increment]
   1    for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
   2    ^~~
   31 warning generated.
   

   https://github.com/bitcoin/bitcoin/blob/aa8d76806c74a51ec66e5004394fe9ea8ff0fac4/src/init.cpp#L968-L972

   If you want to build master, with -Wunreachable-code-loop-increment, you can use something like this:

   0./autogen.sh
   1./configure CXXFLAGS="-Wunreachable-code-loop-increment"
   2make check
   

2. fanquake added the label Refactoring on May 19, 2020
3. fanquake added the label good first issue on May 19, 2020
4. 
   MarcoFalke commented at 2:44 pm on May 19, 2020: member

   Does this issue have a solution, or is this a false positive?
5. 
   practicalswift commented at 3:14 pm on May 19, 2020: contributor

   Does this issue have a solution, or is this a false positive?

   We are currently only reporting the first configuration issue encountered.

   I guess the intention was something along the lines of:

    0diff --git a/src/init.cpp b/src/init.cpp
    1index 3b97ba08d..9cb446e14 100644
    2--- a/src/init.cpp
    3+++ b/src/init.cpp
    4@@ -967,7 +967,10 @@ bool AppInitParameterInteraction()
    5     // on the command line or in this network's section of the config file.
    6     std::string network = gArgs.GetChainName();
    7     for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) {
    8-        return InitError(strprintf(_("Config setting for %s only applied on %s network when in [%s] section."), arg, network, network));
    9+        InitError(strprintf(_("Config setting for %s only applied on %s network when in [%s] section."), arg, network, network));
   10+    }
   11+    if (!gArgs.GetUnsuitableSectionOnlyArgs().empty()) {
   12+        return false;
   13     }
   14
   15     // Warn if unrecognized section name are present in the config file.
   

6. 
   MarcoFalke commented at 3:21 pm on May 19, 2020: member

   InitError should only be called once, otherwise we will end up with the javascript popup hell from 2005.

   

7. 
   ryanofsky commented at 3:28 pm on May 19, 2020: member

   I think it wants to set a cookie
8. 
   MarcoFalke commented at 1:11 pm on May 21, 2020: member

   Back to topic: If someone wants to fix this, the solution is probably to collect all error messages in the loop and then send out one (and only one) InitError. Should be easy to fix unless I am missing something.
9. 
   jonathanschoeller commented at 10:58 pm on May 31, 2020: none

   I can have a look at this.

   Back to topic: If someone wants to fix this, the solution is probably to collect all error messages in the loop and then send out one (and only one) InitError.

   While we’re at it, should we also fix the InitWarning loop directly below to avoid pop-up hell there? https://github.com/bitcoin/bitcoin/blob/aa8d76806c74a51ec66e5004394fe9ea8ff0fac4/src/init.cpp#L975-L977

10. 
    MarcoFalke commented at 11:03 pm on May 31, 2020: member

    @jonathanschoeller Good catch. Please fix all issues you can find :)
11. laanwj closed this on Jun 4, 2020

    ---

12. sidhujag referenced this in commit 2ab90a2fe4 on Jun 4, 2020
13. MarcoFalke locked this on Feb 15, 2022

Contributors
fanquake MarcoFalke practicalswift ryanofsky jonathanschoeller

Labels
Refactoring good first issue