Merge pull request #6284

c6455c7 doc: mention change to option parsing behavior in release notes (Wladimir J. van der Laan) c38c49d Fix argument parsing oddity with -noX (Wladimir J. van der Laan)
bitcoin · Aug 3, 2015 · 10ac38e · 10ac38e
2 parents 9e6c33b + c6455c7
commit 10ac38e
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 39 deletions.
diff --git a/doc/release-notes.md b/doc/release-notes.md
@@ -27,6 +27,14 @@ Low-level RPC API changes
   advantage if a JSON library insists on using a lossy floating point type for
   numbers, which would be dangerous for monetary amounts.
 
+Option parsing behavior
+-----------------------
+
+Command line options are now parsed strictly in the order in which they are
+specified. It used to be the case that `-X -noX` ends up, unintuitively, with X
+set, as `-X` had precedence over `-noX`. This is no longer the case. Like for
+other software, the last specified value for an option will hold.
+
 0.12.0 Change log
 =================
 

diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp
@@ -60,18 +60,18 @@ BOOST_AUTO_TEST_CASE(boolarg)
     BOOST_CHECK(!GetBoolArg("-foo", false));
     BOOST_CHECK(!GetBoolArg("-foo", true));
 
-    ResetArgs("-foo -nofoo");  // -foo should win
+    ResetArgs("-foo -nofoo");  // -nofoo should win
-    BOOST_CHECK(GetBoolArg("-foo", false));
+    BOOST_CHECK(!GetBoolArg("-foo", false));
-    BOOST_CHECK(GetBoolArg("-foo", true));
+    BOOST_CHECK(!GetBoolArg("-foo", true));
-
-    ResetArgs("-foo=1 -nofoo=1");  // -foo should win
-    BOOST_CHECK(GetBoolArg("-foo", false));
-    BOOST_CHECK(GetBoolArg("-foo", true));
 
-    ResetArgs("-foo=0 -nofoo=0");  // -foo should win
+    ResetArgs("-foo=1 -nofoo=1");  // -nofoo should win
     BOOST_CHECK(!GetBoolArg("-foo", false));
     BOOST_CHECK(!GetBoolArg("-foo", true));
 
+    ResetArgs("-foo=0 -nofoo=0");  // -nofoo=0 should win
+    BOOST_CHECK(GetBoolArg("-foo", false));
+    BOOST_CHECK(GetBoolArg("-foo", true));
+
     // New 0.6 feature: treat -- same as -:
     ResetArgs("--foo=1");
     BOOST_CHECK(GetBoolArg("-foo", false));
@@ -150,9 +150,9 @@ BOOST_AUTO_TEST_CASE(boolargno)
     BOOST_CHECK(GetBoolArg("-foo", true));
     BOOST_CHECK(GetBoolArg("-foo", false));
 
-    ResetArgs("-foo --nofoo");
+    ResetArgs("-foo --nofoo"); // --nofoo should win
-    BOOST_CHECK(GetBoolArg("-foo", true));
+    BOOST_CHECK(!GetBoolArg("-foo", true));
-    BOOST_CHECK(GetBoolArg("-foo", false));
+    BOOST_CHECK(!GetBoolArg("-foo", false));
 
     ResetArgs("-nofoo -foo"); // foo always wins:
     BOOST_CHECK(GetBoolArg("-foo", true));

diff --git a/src/util.cpp b/src/util.cpp
@@ -315,18 +315,21 @@ int LogPrintStr(const std::string &str)
     return ret;
 }
 
-static void InterpretNegativeSetting(string name, map<string, string>& mapSettingsRet)
+/** Interpret string as boolean, for argument parsing */
+static bool InterpretBool(const std::string& strValue)
 {
-    // interpret -nofoo as -foo=0 (and -nofoo=0 as -foo=1) as long as -foo not set
+    if (strValue.empty())
-    if (name.find("-no") == 0)
+        return true;
+    return (atoi(strValue) != 0);
+}
+
+/** Turn -noX into -X=0 */
+static void InterpretNegativeSetting(std::string& strKey, std::string& strValue)
+{
+    if (strKey.length()>3 && strKey[0]=='-' && strKey[1]=='n' && strKey[2]=='o')
     {
-        std::string positive("-");
+        strKey = "-" + strKey.substr(3);
-        positive.append(name.begin()+3, name.end());
+        strValue = InterpretBool(strValue) ? "0" : "1";
-        if (mapSettingsRet.count(positive) == 0)
-        {
-            bool value = !GetBoolArg(name, false);
-            mapSettingsRet[positive] = (value ? "1" : "0");
-        }
     }
 }
 
@@ -358,17 +361,11 @@ void ParseParameters(int argc, const char* const argv[])
         // If both --foo and -foo are set, the last takes effect.
         if (str.length() > 1 && str[1] == '-')
             str = str.substr(1);
+        InterpretNegativeSetting(str, strValue);
 
         mapArgs[str] = strValue;
         mapMultiArgs[str].push_back(strValue);
     }
-
-    // New 0.6 features:
-    BOOST_FOREACH(const PAIRTYPE(string,string)& entry, mapArgs)
-    {
-        // interpret -nofoo as -foo=0 (and -nofoo=0 as -foo=1) as long as -foo not set
-        InterpretNegativeSetting(entry.first, mapArgs);
-    }
 }
 
 std::string GetArg(const std::string& strArg, const std::string& strDefault)
@@ -388,11 +385,7 @@ int64_t GetArg(const std::string& strArg, int64_t nDefault)
 bool GetBoolArg(const std::string& strArg, bool fDefault)
 {
     if (mapArgs.count(strArg))
-    {
+        return InterpretBool(mapArgs[strArg]);
-        if (mapArgs[strArg].empty())
-            return true;
-        return (atoi(mapArgs[strArg]) != 0);
-    }
     return fDefault;
 }
 
@@ -543,13 +536,11 @@ void ReadConfigFile(map<string, string>& mapSettingsRet,
     {
         // Don't overwrite existing settings so command line settings override bitcoin.conf
         string strKey = string("-") + it->string_key;
+        string strValue = it->value[0];
+        InterpretNegativeSetting(strKey, strValue);
         if (mapSettingsRet.count(strKey) == 0)
-        {
+            mapSettingsRet[strKey] = strValue;
-            mapSettingsRet[strKey] = it->value[0];
+        mapMultiSettingsRet[strKey].push_back(strValue);
-            // interpret nofoo=1 as foo=0 (and nofoo=0 as foo=1) as long as foo not set)
-            InterpretNegativeSetting(strKey, mapSettingsRet);
-        }
-        mapMultiSettingsRet[strKey].push_back(it->value[0]);
     }
     // If datadir is changed in .conf file:
     ClearDatadirCache();