diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -9,4 +9,7 @@ transactions - Remove miner policy estimator in favor of minimum fees, also remove `fee_estimates.dat`. Old copies will be left in place. - - The log timestamp format is now ISO 8601 (e.g. "2019-01-28T15:41:17Z") \ No newline at end of file + - The log timestamp format is now ISO 8601 (e.g. "2019-01-28T15:41:17Z") + - Behavior change: in case of multiple values for an argument, the following rules apply: + - From the command line, the *last* value takes precedence + - From the config file, the *first* value takes precedence \ No newline at end of file diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -170,16 +170,22 @@ } struct TestArgsManager : public ArgsManager { - std::map &GetMapArgs() { return mapArgs; } - std::map> &GetMapMultiArgs() { - return mapMultiArgs; + std::map> &GetOverrideArgs() { + return m_override_args; + } + std::map> &GetConfigArgs() { + return m_config_args; } const std::unordered_set &GetNegatedArgs() { return m_negated_args; } void ReadConfigString(const std::string str_config) { - std::istringstream stream(str_config); - ReadConfigStream(stream); + std::istringstream streamConfig(str_config); + { + LOCK(cs_args); + m_config_args.clear(); + } + ReadConfigStream(streamConfig); } }; @@ -189,30 +195,33 @@ "-ccc=multiple", "f", "-d=e"}; testArgs.ParseParameters(0, (char **)argv_test); - BOOST_CHECK(testArgs.GetMapArgs().empty() && - testArgs.GetMapMultiArgs().empty()); + BOOST_CHECK(testArgs.GetOverrideArgs().empty() && + testArgs.GetConfigArgs().empty()); testArgs.ParseParameters(1, (char **)argv_test); - BOOST_CHECK(testArgs.GetMapArgs().empty() && - testArgs.GetMapMultiArgs().empty()); + BOOST_CHECK(testArgs.GetOverrideArgs().empty() && + testArgs.GetConfigArgs().empty()); testArgs.ParseParameters(5, (char **)argv_test); // expectation: -ignored is ignored (program name argument), // -a, -b and -ccc end up in map, -d ignored because it is after // a non-option argument (non-GNU option parsing) - BOOST_CHECK(testArgs.GetMapArgs().size() == 3 && - testArgs.GetMapMultiArgs().size() == 3); + BOOST_CHECK(testArgs.GetOverrideArgs().size() == 3 && + testArgs.GetConfigArgs().empty()); BOOST_CHECK(testArgs.IsArgSet("-a") && testArgs.IsArgSet("-b") && testArgs.IsArgSet("-ccc") && !testArgs.IsArgSet("f") && !testArgs.IsArgSet("-d")); - BOOST_CHECK(testArgs.GetMapMultiArgs().count("-a") && - testArgs.GetMapMultiArgs().count("-b") && - testArgs.GetMapMultiArgs().count("-ccc") && - !testArgs.GetMapMultiArgs().count("f") && - !testArgs.GetMapMultiArgs().count("-d")); - - BOOST_CHECK(testArgs.GetMapArgs()["-a"] == "" && - testArgs.GetMapArgs()["-ccc"] == "multiple"); + BOOST_CHECK(testArgs.GetOverrideArgs().count("-a") && + testArgs.GetOverrideArgs().count("-b") && + testArgs.GetOverrideArgs().count("-ccc") && + !testArgs.GetOverrideArgs().count("f") && + !testArgs.GetOverrideArgs().count("-d")); + + BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].size() == 1); + BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].front() == ""); + BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].size() == 2); + BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].front() == "argument"); + BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].back() == "multiple"); BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2); } @@ -228,8 +237,8 @@ } // Nothing else should be in the map - BOOST_CHECK(testArgs.GetMapArgs().size() == 6 && - testArgs.GetMapMultiArgs().size() == 6); + BOOST_CHECK(testArgs.GetOverrideArgs().size() == 6 && + testArgs.GetConfigArgs().empty()); // The -no prefix should get stripped on the way in. BOOST_CHECK(!testArgs.IsArgSet("-nob")); @@ -317,17 +326,17 @@ test_args.ReadConfigString(str_config); // expectation: a, b, ccc, d, fff, ggg, h, i end up in map - BOOST_CHECK(test_args.GetMapArgs().size() == 8); - BOOST_CHECK(test_args.GetMapMultiArgs().size() == 8); + BOOST_CHECK(test_args.GetOverrideArgs().empty()); + BOOST_CHECK(test_args.GetConfigArgs().size() == 8); - BOOST_CHECK(test_args.GetMapArgs().count("-a") && - test_args.GetMapArgs().count("-b") && - test_args.GetMapArgs().count("-ccc") && - test_args.GetMapArgs().count("-d") && - test_args.GetMapArgs().count("-fff") && - test_args.GetMapArgs().count("-ggg") && - test_args.GetMapArgs().count("-h") && - test_args.GetMapArgs().count("-i")); + BOOST_CHECK(test_args.GetConfigArgs().count("-a") && + test_args.GetConfigArgs().count("-b") && + test_args.GetConfigArgs().count("-ccc") && + test_args.GetConfigArgs().count("-d") && + test_args.GetConfigArgs().count("-fff") && + test_args.GetConfigArgs().count("-ggg") && + test_args.GetConfigArgs().count("-h") && + test_args.GetConfigArgs().count("-i")); BOOST_CHECK(test_args.IsArgSet("-a") && test_args.IsArgSet("-b") && test_args.IsArgSet("-ccc") && test_args.IsArgSet("-d") && @@ -398,16 +407,24 @@ BOOST_AUTO_TEST_CASE(util_GetArg) { TestArgsManager testArgs; - testArgs.GetMapArgs().clear(); - testArgs.GetMapArgs()["strtest1"] = "string..."; + testArgs.GetOverrideArgs().clear(); + testArgs.GetOverrideArgs()["strtest1"] = {"string..."}; // strtest2 undefined on purpose - testArgs.GetMapArgs()["inttest1"] = "12345"; - testArgs.GetMapArgs()["inttest2"] = "81985529216486895"; + testArgs.GetOverrideArgs()["inttest1"] = {"12345"}; + testArgs.GetOverrideArgs()["inttest2"] = {"81985529216486895"}; // inttest3 undefined on purpose - testArgs.GetMapArgs()["booltest1"] = ""; + testArgs.GetOverrideArgs()["booltest1"] = {""}; // booltest2 undefined on purpose - testArgs.GetMapArgs()["booltest3"] = "0"; - testArgs.GetMapArgs()["booltest4"] = "1"; + testArgs.GetOverrideArgs()["booltest3"] = {"0"}; + testArgs.GetOverrideArgs()["booltest4"] = {"1"}; + + // priorities + testArgs.GetOverrideArgs()["pritest1"] = {"a", "b"}; + testArgs.GetConfigArgs()["pritest2"] = {"a", "b"}; + testArgs.GetOverrideArgs()["pritest3"] = {"a"}; + testArgs.GetConfigArgs()["pritest3"] = {"b"}; + testArgs.GetOverrideArgs()["pritest4"] = {"a", "b"}; + testArgs.GetConfigArgs()["pritest4"] = {"c", "d"}; BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); @@ -418,27 +435,31 @@ BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest2", false), false); BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest3", false), false); BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest4", false), true); + + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest1", "default"), "b"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest2", "default"), "a"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest3", "default"), "a"); + BOOST_CHECK_EQUAL(testArgs.GetArg("pritest4", "default"), "b"); } BOOST_AUTO_TEST_CASE(util_ClearArg) { TestArgsManager testArgs; // Clear single string arg - testArgs.GetMapArgs()["strtest1"] = "string..."; + testArgs.GetOverrideArgs()["strtest1"] = {"string..."}; BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string..."); testArgs.ClearArg("strtest1"); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "default"); // Clear boolean arg - testArgs.GetMapArgs()["booltest1"] = "1"; + testArgs.GetOverrideArgs()["booltest1"] = {"1"}; BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest1", false), true); testArgs.ClearArg("booltest1"); BOOST_CHECK_EQUAL(testArgs.GetArg("booltest1", false), false); - // Clear multi args only - testArgs.GetMapMultiArgs()["strtest2"].push_back("string..."); - testArgs.GetMapMultiArgs()["strtest2"].push_back("...gnirts"); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); + // Clear config args only + testArgs.GetConfigArgs()["strtest2"].push_back("string..."); + testArgs.GetConfigArgs()["strtest2"].push_back("...gnirts"); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 2); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").back(), "...gnirts"); @@ -446,13 +467,14 @@ BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 0); - // Clear both arg and multi args - testArgs.GetMapArgs()["strtest3"] = "string..."; - testArgs.GetMapMultiArgs()["strtest3"].push_back("string..."); - testArgs.GetMapMultiArgs()["strtest3"].push_back("...gnirts"); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest3", "default"), "string..."); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").size(), 2); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").front(), "string..."); + // Clear both cli args and config args + testArgs.GetOverrideArgs()["strtest3"].push_back("cli string..."); + testArgs.GetOverrideArgs()["strtest3"].push_back("...gnirts ilc"); + testArgs.GetConfigArgs()["strtest3"].push_back("string..."); + testArgs.GetConfigArgs()["strtest3"].push_back("...gnirts"); + BOOST_CHECK_EQUAL(testArgs.GetArg("strtest3", "default"), "...gnirts ilc"); + BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").size(), 4); + BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").front(), "cli string..."); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").back(), "...gnirts"); testArgs.ClearArg("strtest3"); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest3", "default"), "default"); @@ -497,7 +519,7 @@ // ForceSetMultiArg testArgs.ForceSetMultiArg("strtest2", "string..."); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "...gnirts"); + BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 2); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "...gnirts"); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").back(), "string..."); @@ -509,7 +531,8 @@ BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 1); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "string..."); testArgs.ForceSetMultiArg("strtest2", "one more thing..."); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "string..."); + BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), + "one more thing..."); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 2); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").back(), "one more thing..."); diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -95,9 +95,11 @@ class ArgsManager { protected: + friend class ArgsManagerHelper; + mutable CCriticalSection cs_args; - std::map mapArgs; - std::map> mapMultiArgs; + std::map> m_override_args; + std::map> m_config_args; std::unordered_set m_negated_args; void ReadConfigStream(std::istream &stream); diff --git a/src/util.cpp b/src/util.cpp --- a/src/util.cpp +++ b/src/util.cpp @@ -168,6 +168,71 @@ return (atoi(strValue) != 0); } +/** Internal helper functions for ArgsManager */ +class ArgsManagerHelper { +public: + typedef std::map> MapArgs; + + /** Find arguments in a map and add them to a vector */ + static inline void AddArgs(std::vector &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 + 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 GetArg(const ArgsManager &am, + const std::string &arg) { + LOCK(am.cs_args); + std::pair 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. * @@ -198,8 +263,7 @@ 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++) { @@ -229,23 +293,21 @@ // 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 ArgsManager::GetArgs(const std::string &strArg) const { + std::vector 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 { @@ -255,28 +317,28 @@ 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 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 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 found_res = + ArgsManagerHelper::GetArg(*this, strArg); + if (found_res.first) { + return InterpretBool(found_res.second); } return fDefault; } @@ -302,8 +364,7 @@ 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}; } /** @@ -314,16 +375,13 @@ void ArgsManager::ForceSetMultiArg(const std::string &strArg, const std::string &strValue) { LOCK(cs_args); - if (mapArgs.count(strArg) == 0) { - mapArgs[strArg] = strValue; - } - mapMultiArgs[strArg].push_back(strValue); + m_override_args[strArg].push_back(strValue); } void ArgsManager::ClearArg(const std::string &strArg) { LOCK(cs_args); - mapArgs.erase(strArg); - mapMultiArgs.erase(strArg); + m_override_args.erase(strArg); + m_config_args.erase(strArg); } bool HelpRequested(const ArgsManager &args) { @@ -464,14 +522,16 @@ 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