ArgsManager: keep command line and config file arguments separate

1 files changed, 82 insertions, 23 deletions

diff --git a/src/util.cpp b/src/util.cpp
index f55c9c8c34..f8366fe694 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -455,6 +455,67 @@ static bool InterpretBool(const std::string& strValue)
     return (atoi(strValue) != 0);
 }
 
+/** Internal helper functions for ArgsManager */
+class ArgsManagerHelper {
+public:
+    typedef std::map<std::string, std::vector<std::string>> MapArgs;
+
+    /** Find arguments in a map and add them to a vector */
+    static inline void AddArgs(std::vector<std::string>& res, const MapArgs& map_args, const std::string& arg)
+    {
+        auto it = map_args.find(arg);
+        if (it != map_args.end()) {
+            res.insert(res.end(), it->second.begin(), it->second.end());
+        }
+    }
+
+    /** Return true/false if an argument is set in a map, and also
+     *  return the first (or last) of the possibly multiple values it has
+     */
+    static inline std::pair<bool,std::string> GetArgHelper(const MapArgs& map_args, const std::string& arg, bool getLast = false)
+    {
+        auto it = map_args.find(arg);
+
+        if (it == map_args.end() || it->second.empty()) {
+            return std::make_pair(false, std::string());
+        }
+
+        if (getLast) {
+            return std::make_pair(true, it->second.back());
+        } else {
+            return std::make_pair(true, it->second.front());
+        }
+    }
+
+    /* Get the string value of an argument, returning a pair of a boolean
+     * indicating the argument was found, and the value for the argument
+     * if it was found (or the empty string if not found).
+     */
+    static inline std::pair<bool,std::string> GetArg(const ArgsManager &am, const std::string& arg)
+    {
+        LOCK(am.cs_args);
+        std::pair<bool,std::string> found_result(false, std::string());
+
+        // We pass "true" to GetArgHelper in order to return the last
+        // argument value seen from the command line (so "bitcoind -foo=bar
+        // -foo=baz" gives GetArg(am,"foo")=={true,"baz"}
+        found_result = GetArgHelper(am.m_override_args, arg, true);
+        if (found_result.first) {
+            return found_result;
+        }
+
+        // But in contrast we return the first argument seen in a config file,
+        // so "foo=bar \n foo=baz" in the config file gives
+        // GetArg(am,"foo")={true,"bar"}
+        found_result = GetArgHelper(am.m_config_args, arg);
+        if (found_result.first) {
+            return found_result;
+        }
+
+        return found_result;
+    }
+};
+
 /**
  * Interpret -nofoo as if the user supplied -foo=0.
  *
@@ -485,8 +546,7 @@ void ArgsManager::InterpretNegatedOption(std::string& key, std::string& val)
 void ArgsManager::ParseParameters(int argc, const char* const argv[])
 {
     LOCK(cs_args);
-    mapArgs.clear();
-    mapMultiArgs.clear();
+    m_override_args.clear();
     m_negated_args.clear();
 
     for (int i = 1; i < argc; i++) {
@@ -513,23 +573,23 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
         // Transform -nofoo to -foo=0
         InterpretNegatedOption(key, val);
 
-        mapArgs[key] = val;
-        mapMultiArgs[key].push_back(val);
+        m_override_args[key].push_back(val);
     }
 }
 
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
 {
+    std::vector<std::string> result = {};
+
     LOCK(cs_args);
-    auto it = mapMultiArgs.find(strArg);
-    if (it != mapMultiArgs.end()) return it->second;
-    return {};
+    ArgsManagerHelper::AddArgs(result, m_override_args, strArg);
+    ArgsManagerHelper::AddArgs(result, m_config_args, strArg);
+    return result;
 }
 
 bool ArgsManager::IsArgSet(const std::string& strArg) const
 {
-    LOCK(cs_args);
-    return mapArgs.count(strArg);
+    return ArgsManagerHelper::GetArg(*this, strArg).first;
 }
 
 bool ArgsManager::IsArgNegated(const std::string& strArg) const
@@ -540,25 +600,22 @@ bool ArgsManager::IsArgNegated(const std::string& strArg) const
 
 std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const
 {
-    LOCK(cs_args);
-    auto it = mapArgs.find(strArg);
-    if (it != mapArgs.end()) return it->second;
+    std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
+    if (found_res.first) return found_res.second;
     return strDefault;
 }
 
 int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
 {
-    LOCK(cs_args);
-    auto it = mapArgs.find(strArg);
-    if (it != mapArgs.end()) return atoi64(it->second);
+    std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
+    if (found_res.first) return atoi64(found_res.second);
     return nDefault;
 }
 
 bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
 {
-    LOCK(cs_args);
-    auto it = mapArgs.find(strArg);
-    if (it != mapArgs.end()) return InterpretBool(it->second);
+    std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
+    if (found_res.first) return InterpretBool(found_res.second);
     return fDefault;
 }
 
@@ -581,8 +638,7 @@ bool ArgsManager::SoftSetBoolArg(const std::string& strArg, bool fValue)
 void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strValue)
 {
     LOCK(cs_args);
-    mapArgs[strArg] = strValue;
-    mapMultiArgs[strArg] = {strValue};
+    m_override_args[strArg] = {strValue};
 }
 
 bool HelpRequested(const ArgsManager& args)
@@ -749,14 +805,17 @@ void ArgsManager::ReadConfigStream(std::istream& stream)
         std::string strKey = std::string("-") + it->string_key;
         std::string strValue = it->value[0];
         InterpretNegatedOption(strKey, strValue);
-        if (mapArgs.count(strKey) == 0)
-            mapArgs[strKey] = strValue;
-        mapMultiArgs[strKey].push_back(strValue);
+        m_config_args[strKey].push_back(strValue);
     }
 }
 
 void ArgsManager::ReadConfigFile(const std::string& confPath)
 {
+    {
+        LOCK(cs_args);
+        m_config_args.clear();
+    }
+
     fs::ifstream stream(GetConfigFile(confPath));
 
     // ok to not have a config file