ArgsManager: drop m_negated_args

When a -nofoo option is seen, instead of adding it to a separate
set of negated args, set the arg as being an empty vector of strings.

This changes the behaviour in some ways:
 - -nofoo=0 still sets foo=1 but no longer treats it as a negated arg
 - -nofoo=1 -foo=2 has GetArgs() return [2] rather than [2,0]
 - "foo=2 \n -nofoo=1" in a config file no longer returns [2,0], just [0]
 - GetArgs returns an empty vector for negated args

src/util.cpp

@@ -519,35 +519,44 @@ public:
 /**
  * Interpret -nofoo as if the user supplied -foo=0.
  *
- * This method also tracks when the -no form was supplied, and treats "-foo" as
- * a negated option when this happens. This can be later checked using the
+ * This method also tracks when the -no form was supplied, and if so,
+ * checks whether there was a double-negative (-nofoo=0 -> -foo=1).
+ *
+ * If there was not a double negative, it removes the "no" from the key,
+ * and returns true, indicating the caller should clear the args vector
+ * to indicate a negated option.
+ *
+ * If there was a double negative, it removes "no" from the key, sets the
+ * value to "1" and returns false.
+ *
+ * If there was no "no", it leaves key and value untouched and returns
+ * false.
+ *
+ * Where an option was negated can be later checked using the
  * IsArgNegated() method. One use case for this is to have a way to disable
  * options that are not normally boolean (e.g. using -nodebuglogfile to request
  * that debug log output is not sent to any file at all).
  */
-void ArgsManager::InterpretNegatedOption(std::string& key, std::string& val)
+static bool InterpretNegatedOption(std::string& key, std::string& val)
 {
     if (key.substr(0, 3) == "-no") {
         bool bool_val = InterpretBool(val);
+        key.erase(1, 2);
         if (!bool_val ) {
             // Double negatives like -nofoo=0 are supported (but discouraged)
             LogPrintf("Warning: parsed potentially confusing double-negative %s=%s\n", key, val);
+            val = "1";
+        } else {
+            return true;
         }
-        key.erase(1, 2);
-        m_negated_args.insert(key);
-        val = bool_val ? "0" : "1";
-    } else {
-        // In an invocation like "bitcoind -nofoo -foo" we want to unmark -foo
-        // as negated when we see the second option.
-        m_negated_args.erase(key);
     }
+    return false;
 }
 
 void ArgsManager::ParseParameters(int argc, const char* const argv[])
 {
     LOCK(cs_args);
     m_override_args.clear();
-    m_negated_args.clear();
 
     for (int i = 1; i < argc; i++) {
         std::string key(argv[i]);
@@ -570,16 +579,19 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
         if (key.length() > 1 && key[1] == '-')
             key.erase(0, 1);
 
-        // Transform -nofoo to -foo=0
-        InterpretNegatedOption(key, val);
-
-        m_override_args[key].push_back(val);
+        // Check for -nofoo
+        if (InterpretNegatedOption(key, val)) {
+            m_override_args[key].clear();
+        } else {
+            m_override_args[key].push_back(val);
+        }
     }
 }
 
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 {
     std::vector<std::string> result = {};
+    if (IsArgNegated(strArg)) return result; // special case
 
     LOCK(cs_args);
     ArgsManagerHelper::AddArgs(result, m_override_args, strArg);
@@ -589,17 +601,26 @@ std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 
 bool ArgsManager::IsArgSet(const std::string& strArg) const
 {
+    if (IsArgNegated(strArg)) return true; // special case
     return ArgsManagerHelper::GetArg(*this, strArg).first;
 }
 
 bool ArgsManager::IsArgNegated(const std::string& strArg) const
 {
     LOCK(cs_args);
-    return m_negated_args.find(strArg) != m_negated_args.end();
+
+    const auto& ov = m_override_args.find(strArg);
+    if (ov != m_override_args.end()) return ov->second.empty();
+
+    const auto& cf = m_config_args.find(strArg);
+    if (cf != m_config_args.end()) return cf->second.empty();
+
+    return false;
 }
 
 std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const
 {
+    if (IsArgNegated(strArg)) return "0";
     std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
     if (found_res.first) return found_res.second;
     return strDefault;
@@ -607,6 +628,7 @@ std::string ArgsManager::GetArg(const std::string& strArg, const std::string& st
 
 int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
 {
+    if (IsArgNegated(strArg)) return 0;
     std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
     if (found_res.first) return atoi64(found_res.second);
     return nDefault;
@@ -614,6 +636,7 @@ int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
 
 bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
 {
+    if (IsArgNegated(strArg)) return false;
     std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
     if (found_res.first) return InterpretBool(found_res.second);
     return fDefault;
@@ -801,11 +824,13 @@ void ArgsManager::ReadConfigStream(std::istream& stream)
 
     for (boost::program_options::detail::config_file_iterator it(stream, setOptions), end; it != end; ++it)
     {
-        // Don't overwrite existing settings so command line settings override bitcoin.conf
         std::string strKey = std::string("-") + it->string_key;
         std::string strValue = it->value[0];
-        InterpretNegatedOption(strKey, strValue);
-        m_config_args[strKey].push_back(strValue);
+        if (InterpretNegatedOption(strKey, strValue)) {
+            m_config_args[strKey].clear();
+        } else {
+            m_config_args[strKey].push_back(strValue);
+        }
     }
 }