diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -6,6 +6,8 @@ #include +#include + #include #include diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -214,7 +214,7 @@ config.SetMaxBlockSize(8000000); const std::string uacomment = "A very nice comment"; - gArgs.ForceSetMultiArg("-uacomment", uacomment); + gArgs.ForceSetMultiArg("-uacomment", {uacomment}); const std::string versionMessage = "/Bitcoin ABC:" + std::to_string(CLIENT_VERSION_MAJOR) + "." + 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 @@ -13,6 +13,8 @@ #include +#include + #include #include @@ -171,17 +173,11 @@ struct TestArgsManager : public ArgsManager { TestArgsManager() { m_network_only_args.clear(); } - std::map> &GetOverrideArgs() { - return m_override_args; - } - std::map> &GetConfigArgs() { - return m_config_args; - } void ReadConfigString(const std::string str_config) { std::istringstream streamConfig(str_config); { LOCK(cs_args); - m_config_args.clear(); + m_settings.ro_config.clear(); m_config_sections.clear(); } std::string error; @@ -199,6 +195,7 @@ } using ArgsManager::cs_args; using ArgsManager::m_network; + using ArgsManager::m_settings; using ArgsManager::ReadConfigStream; }; @@ -213,35 +210,41 @@ "-ccc=multiple", "f", "-d=e"}; std::string error; + LOCK(testArgs.cs_args); testArgs.SetupArgs({a, b, ccc, d}); BOOST_CHECK(testArgs.ParseParameters(0, (char **)argv_test, error)); - BOOST_CHECK(testArgs.GetOverrideArgs().empty() && - testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && + testArgs.m_settings.ro_config.empty()); BOOST_CHECK(testArgs.ParseParameters(1, (char **)argv_test, error)); - BOOST_CHECK(testArgs.GetOverrideArgs().empty() && - testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && + testArgs.m_settings.ro_config.empty()); BOOST_CHECK(testArgs.ParseParameters(7, (char **)argv_test, error)); // 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.GetOverrideArgs().size() == 3 && - testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 3 && + testArgs.m_settings.ro_config.empty()); BOOST_CHECK(testArgs.IsArgSet("-a") && testArgs.IsArgSet("-b") && testArgs.IsArgSet("-ccc") && !testArgs.IsArgSet("f") && !testArgs.IsArgSet("-d")); - 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.m_settings.command_line_options.count("a") && + testArgs.m_settings.command_line_options.count("b") && + testArgs.m_settings.command_line_options.count("ccc") && + !testArgs.m_settings.command_line_options.count("f") && + !testArgs.m_settings.command_line_options.count("d")); + + BOOST_CHECK(testArgs.m_settings.command_line_options["a"].size() == 1); + BOOST_CHECK( + testArgs.m_settings.command_line_options["a"].front().get_str() == ""); + BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].size() == 2); + BOOST_CHECK( + testArgs.m_settings.command_line_options["ccc"].front().get_str() == + "argument"); + BOOST_CHECK( + testArgs.m_settings.command_line_options["ccc"].back().get_str() == + "multiple"); BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2); } @@ -298,6 +301,7 @@ const char *argv_test[] = {"ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"}; std::string error; + LOCK(testArgs.cs_args); testArgs.SetupArgs({a, b, c, d, e, f}); BOOST_CHECK(testArgs.ParseParameters(7, (char **)argv_test, error)); @@ -307,8 +311,8 @@ } // Nothing else should be in the map - BOOST_CHECK(testArgs.GetOverrideArgs().size() == 6 && - testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 6 && + testArgs.m_settings.ro_config.empty()); // The -no prefix should get stripped on the way in. BOOST_CHECK(!testArgs.IsArgSet("-nob")); @@ -401,6 +405,7 @@ "iii=2\n"; TestArgsManager test_args; + LOCK(test_args.cs_args); const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL); const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL); const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_STRING); @@ -417,21 +422,24 @@ // expectation: a, b, ccc, d, fff, ggg, h, i end up in map // so do sec1.ccc, sec1.d, sec1.h, sec2.ccc, sec2.iii - BOOST_CHECK(test_args.GetOverrideArgs().empty()); - BOOST_CHECK(test_args.GetConfigArgs().size() == 13); - - 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.GetConfigArgs().count("-sec1.ccc") && - test_args.GetConfigArgs().count("-sec1.h") && - test_args.GetConfigArgs().count("-sec2.ccc") && - test_args.GetConfigArgs().count("-sec2.iii")); + BOOST_CHECK(test_args.m_settings.command_line_options.empty()); + BOOST_CHECK(test_args.m_settings.ro_config.size() == 3); + BOOST_CHECK(test_args.m_settings.ro_config[""].size() == 8); + BOOST_CHECK(test_args.m_settings.ro_config["sec1"].size() == 3); + BOOST_CHECK(test_args.m_settings.ro_config["sec2"].size() == 2); + + BOOST_CHECK(test_args.m_settings.ro_config[""].count("a") && + test_args.m_settings.ro_config[""].count("b") && + test_args.m_settings.ro_config[""].count("ccc") && + test_args.m_settings.ro_config[""].count("d") && + test_args.m_settings.ro_config[""].count("fff") && + test_args.m_settings.ro_config[""].count("ggg") && + test_args.m_settings.ro_config[""].count("h") && + test_args.m_settings.ro_config[""].count("i")); + BOOST_CHECK(test_args.m_settings.ro_config["sec1"].count("ccc") && + test_args.m_settings.ro_config["sec1"].count("h") && + test_args.m_settings.ro_config["sec2"].count("ccc") && + test_args.m_settings.ro_config["sec2"].count("iii")); BOOST_CHECK(test_args.IsArgSet("-a") && test_args.IsArgSet("-b") && test_args.IsArgSet("-ccc") && test_args.IsArgSet("-d") && @@ -566,24 +574,26 @@ BOOST_AUTO_TEST_CASE(util_GetArg) { TestArgsManager testArgs; - testArgs.GetOverrideArgs().clear(); - testArgs.GetOverrideArgs()["strtest1"] = {"string..."}; + LOCK(testArgs.cs_args); + testArgs.m_settings.command_line_options.clear(); + testArgs.m_settings.command_line_options["strtest1"] = {"string..."}; // strtest2 undefined on purpose - testArgs.GetOverrideArgs()["inttest1"] = {"12345"}; - testArgs.GetOverrideArgs()["inttest2"] = {"81985529216486895"}; + testArgs.m_settings.command_line_options["inttest1"] = {"12345"}; + testArgs.m_settings.command_line_options["inttest2"] = { + "81985529216486895"}; // inttest3 undefined on purpose - testArgs.GetOverrideArgs()["booltest1"] = {""}; + testArgs.m_settings.command_line_options["booltest1"] = {""}; // booltest2 undefined on purpose - testArgs.GetOverrideArgs()["booltest3"] = {"0"}; - testArgs.GetOverrideArgs()["booltest4"] = {"1"}; + testArgs.m_settings.command_line_options["booltest3"] = {"0"}; + testArgs.m_settings.command_line_options["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"}; + testArgs.m_settings.command_line_options["pritest1"] = {"a", "b"}; + testArgs.m_settings.ro_config[""]["pritest2"] = {"a", "b"}; + testArgs.m_settings.command_line_options["pritest3"] = {"a"}; + testArgs.m_settings.ro_config[""]["pritest3"] = {"b"}; + testArgs.m_settings.command_line_options["pritest4"] = {"a", "b"}; + testArgs.m_settings.ro_config[""]["pritest4"] = {"c", "d"}; BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); @@ -603,42 +613,25 @@ BOOST_AUTO_TEST_CASE(util_ClearForcedArg) { TestArgsManager testArgs; - - // Clear single string arg - testArgs.GetOverrideArgs()["strtest1"] = {"string..."}; - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string..."); - testArgs.ClearForcedArg("strtest1"); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "default"); - - // Clear boolean arg - testArgs.GetOverrideArgs()["booltest1"] = {"1"}; - BOOST_CHECK_EQUAL(testArgs.GetBoolArg("booltest1", false), true); - testArgs.ClearForcedArg("booltest1"); - BOOST_CHECK_EQUAL(testArgs.GetArg("booltest1", false), false); - - // Clear multi arg. - testArgs.GetOverrideArgs()["strtest2"].push_back("string..."); - testArgs.GetOverrideArgs()["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"); - testArgs.ClearForcedArg("strtest2"); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 0); - - // Clear exlusively overridden 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.ClearForcedArg("strtest3"); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").size(), 2); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").front(), "string..."); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest3").back(), "...gnirts"); + LOCK(testArgs.cs_args); + + // Clear command line arg + testArgs.m_settings.command_line_options["cmdarg"] = {"cmdval"}; + BOOST_CHECK_EQUAL(testArgs.GetArg("cmdarg", "default"), "cmdval"); + testArgs.ClearForcedArg("cmdarg"); + BOOST_CHECK_EQUAL(testArgs.GetArg("cmdarg", "default"), "cmdval"); + + // Clear config arg + testArgs.m_settings.ro_config[""]["configarg"] = {"configval"}; + BOOST_CHECK_EQUAL(testArgs.GetArg("configarg", "default"), "configval"); + testArgs.ClearForcedArg("configarg"); + BOOST_CHECK_EQUAL(testArgs.GetArg("configarg", "default"), "configval"); + + // Clear forced arg + testArgs.m_settings.forced_settings["forcedarg"] = {"forcedval"}; + BOOST_CHECK_EQUAL(testArgs.GetArg("forcedarg", "default"), "forcedval"); + testArgs.ClearForcedArg("forcedarg"); + BOOST_CHECK_EQUAL(testArgs.GetArg("forcedarg", "default"), "default"); } BOOST_AUTO_TEST_CASE(util_SetArg) { @@ -678,25 +671,22 @@ BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "...gnirts"); // ForceSetMultiArg - testArgs.ForceSetMultiArg("strtest2", "string..."); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "string..."); + testArgs.ForceSetMultiArg("strtest2", {"string...", "...gnirts"}); + BOOST_CHECK_THROW(testArgs.GetArg("strtest2", "default"), + std::runtime_error); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 2); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "...gnirts"); - BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").back(), "string..."); + BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").front(), "string..."); + BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").back(), "...gnirts"); testArgs.ClearForcedArg("strtest2"); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 0); - testArgs.ForceSetMultiArg("strtest2", "string..."); - BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "string..."); + + // If there are multi args, ForceSetArg should erase them + testArgs.ForceSetMultiArg("strtest2", {"string..."}); + BOOST_CHECK_THROW(testArgs.GetArg("strtest2", "default"), + std::runtime_error); 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"), - "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..."); - // If there are multi args, ForceSetArg should erase them testArgs.ForceSetArg("strtest2", "...gnirts"); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "...gnirts"); BOOST_CHECK_EQUAL(testArgs.GetArgs("strtest2").size(), 1); diff --git a/src/util/system.h b/src/util/system.h --- a/src/util/system.h +++ b/src/util/system.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -169,10 +170,7 @@ }; mutable RecursiveMutex cs_args; - std::map> - m_override_args GUARDED_BY(cs_args); - std::map> - m_config_args GUARDED_BY(cs_args); + util::Settings m_settings GUARDED_BY(cs_args); std::string m_network GUARDED_BY(cs_args); std::set m_network_only_args GUARDED_BY(cs_args); std::map> @@ -285,7 +283,7 @@ // Forces a multi arg setting, used only in testing void ForceSetMultiArg(const std::string &strArg, - const std::string &strValue); + const std::vector &values); /** * Looks for -regtest, -testnet and returns the appropriate BIP70 chain diff --git a/src/util/system.cpp b/src/util/system.cpp --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -17,6 +17,8 @@ #include #include +#include + #include #include @@ -181,11 +183,13 @@ return (atoi(strValue) != 0); } +static std::string SettingName(const std::string &arg) { + return arg.size() > 0 && arg[0] == '-' ? arg.substr(1) : arg; +} + /** Internal helper functions for ArgsManager */ class ArgsManagerHelper { public: - typedef std::map> MapArgs; - /** Determine whether to use config settings in the default section, * See also comments around ArgsManager::ArgsManager() below. */ static inline bool UseDefaultSection(const ArgsManager &am, @@ -195,98 +199,12 @@ am.m_network_only_args.count(arg) == 0); } - /** Convert regular argument into the network-specific setting */ - static inline std::string NetworkArg(const ArgsManager &am, - const std::string &arg) { - assert(arg.length() > 1 && arg[0] == '-'); - return "-" + am.m_network + "." + arg.substr(1); - } - - /** 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) { + static util::SettingsValue Get(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"} - if (!am.m_network.empty()) { - found_result = GetArgHelper(am.m_config_args, NetworkArg(am, arg)); - if (found_result.first) { - return found_result; - } - } - - if (UseDefaultSection(am, arg)) { - found_result = GetArgHelper(am.m_config_args, arg); - if (found_result.first) { - return found_result; - } - } - - return found_result; - } - - /* Special test for -testnet and -regtest args, because we don't want to be - * confused by craziness like "[regtest] testnet=1" - */ - static inline bool GetNetBoolArg(const ArgsManager &am, - const std::string &net_arg) - EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) { - std::pair found_result(false, std::string()); - found_result = GetArgHelper(am.m_override_args, net_arg, true); - if (!found_result.first) { - found_result = GetArgHelper(am.m_config_args, net_arg, true); - if (!found_result.first) { - // not set - return false; - } - } - // is set, so evaluate - return InterpretBool(found_result.second); + return GetSetting(am.m_settings, am.m_network, SettingName(arg), + !UseDefaultSection(am, arg), + /* get_chain_name= */ false); } }; @@ -297,13 +215,12 @@ * 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 clears the args vector to indicate a negated option. + * and returns false. * - * If there was a double negative, it removes "no" from the key, sets the - * value to "1" and pushes the key and the updated value to the args vector. + * If there was a double negative, it removes "no" from the key, and + * returns true. * - * If there was no "no", it leaves key and value untouched and pushes them - * to the args vector. + * If there was no "no", it returns the string value untouched. * * 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 @@ -311,38 +228,44 @@ * output is not sent to any file at all). */ -NODISCARD static bool -InterpretOption(std::string key, std::string val, unsigned int flags, - std::map> &args, - std::string &error) { - assert(key[0] == '-'); - +static util::SettingsValue InterpretOption(std::string §ion, + std::string &key, + const std::string &value) { + // Split section name from key name for keys like "testnet.foo" or + // "regtest.bar" size_t option_index = key.find('.'); - if (option_index == std::string::npos) { - option_index = 1; - } else { - ++option_index; - } - if (key.substr(option_index, 2) == "no") { - key.erase(option_index, 2); - if (flags & ArgsManager::ALLOW_BOOL) { - if (InterpretBool(val)) { - args[key].clear(); - return true; - } - // Double negatives like -nofoo=0 are supported (but discouraged) - LogPrintf( - "Warning: parsed potentially confusing double-negative %s=%s\n", - key, val); - val = "1"; - } else { - error = strprintf( - "Negating of %s is meaningless and therefore forbidden", - key.c_str()); - return false; + if (option_index != std::string::npos) { + section = key.substr(0, option_index); + key.erase(0, option_index + 1); + } + if (key.substr(0, 2) == "no") { + key.erase(0, 2); + // Double negatives like -nofoo=0 are supported (but discouraged) + if (!InterpretBool(value)) { + LogPrintf("Warning: parsed potentially confusing double-negative " + "-%s=%s\n", + key, value); + return true; } + return false; + } + return value; +} + +/** + * Check settings value validity according to flags. + * + * TODO: Add more meaningful error checks here in the future + * See "here's how the flags are meant to behave" in + * https://github.com/bitcoin/bitcoin/pull/16097#issuecomment-514627823 + */ +static bool CheckValid(const std::string &key, const util::SettingsValue &val, + unsigned int flags, std::string &error) { + if (val.isBool() && !(flags & ArgsManager::ALLOW_BOOL)) { + error = strprintf( + "Negating of -%s is meaningless and therefore forbidden", key); + return false; } - args[key].push_back(val); return true; } @@ -366,29 +289,10 @@ } for (const auto &arg : m_network_only_args) { - std::pair found_result; - - // if this option is overridden it's fine - found_result = ArgsManagerHelper::GetArgHelper(m_override_args, arg); - if (found_result.first) { - continue; - } - - // if there's a network-specific value for this option, it's fine - found_result = ArgsManagerHelper::GetArgHelper( - m_config_args, ArgsManagerHelper::NetworkArg(*this, arg)); - if (found_result.first) { - continue; - } - - // if there isn't a default value for this option, it's fine - found_result = ArgsManagerHelper::GetArgHelper(m_config_args, arg); - if (!found_result.first) { - continue; + if (OnlyHasDefaultSectionSetting(m_settings, m_network, + SettingName(arg))) { + unsuitables.insert(arg); } - - // otherwise, issue a warning - unsuitables.insert(arg); } return unsuitables; } @@ -440,7 +344,7 @@ bool ArgsManager::ParseParameters(int argc, const char *const argv[], std::string &error) { LOCK(cs_args); - m_override_args.clear(); + m_settings.command_line_options.clear(); for (int i = 1; i < argc; i++) { std::string key(argv[i]); @@ -453,50 +357,46 @@ break; } + // Transform -foo to foo + key.erase(0, 1); + std::string section; + util::SettingsValue value = InterpretOption(section, key, val); const unsigned int flags = FlagsOfKnownArg(key); if (flags) { - if (!InterpretOption(key, val, flags, m_override_args, error)) { + if (!CheckValid(key, value, flags, error)) { return false; } + // Weird behavior preserved for backwards compatibility: command + // line options with section prefixes are allowed but ignored. It + // would be better if these options triggered the Invalid parameter + // error below. + if (section.empty()) { + m_settings.command_line_options[key].push_back(value); + } } else { - error = strprintf("Invalid parameter %s", key.c_str()); + error = strprintf("Invalid parameter -%s", key); return false; } } // we do not allow -includeconf from command line, so we clear it here - auto it = m_override_args.find("-includeconf"); - if (it != m_override_args.end()) { - if (it->second.size() > 0) { - for (const auto &ic : it->second) { - error += "-includeconf cannot be used from commandline; " - "-includeconf=" + - ic + "\n"; - } - return false; + bool success = true; + if (auto *includes = + util::FindKey(m_settings.command_line_options, "includeconf")) { + for (const auto &include : util::SettingsSpan(*includes)) { + error += + "-includeconf cannot be used from commandline; -includeconf=" + + include.get_str() + "\n"; + success = false; } } - return true; + return success; } unsigned int ArgsManager::FlagsOfKnownArg(const std::string &key) const { - assert(key[0] == '-'); - - size_t option_index = key.find('.'); - if (option_index == std::string::npos) { - option_index = 1; - } else { - ++option_index; - } - if (key.substr(option_index, 2) == "no") { - option_index += 2; - } - - const std::string base_arg_name = '-' + key.substr(option_index); - LOCK(cs_args); for (const auto &arg_map : m_available_args) { - const auto search = arg_map.second.find(base_arg_name); + const auto search = arg_map.second.find('-' + key); if (search != arg_map.second.end()) { return search->second.m_flags; } @@ -505,95 +405,51 @@ } std::vector ArgsManager::GetArgs(const std::string &strArg) const { - std::vector result = {}; - // special case - if (IsArgNegated(strArg)) { - return result; - } - LOCK(cs_args); - - ArgsManagerHelper::AddArgs(result, m_override_args, strArg); - if (!m_network.empty()) { - ArgsManagerHelper::AddArgs( - result, m_config_args, - ArgsManagerHelper::NetworkArg(*this, strArg)); - } - - if (ArgsManagerHelper::UseDefaultSection(*this, strArg)) { - ArgsManagerHelper::AddArgs(result, m_config_args, strArg); + bool ignore_default_section_config = + !ArgsManagerHelper::UseDefaultSection(*this, strArg); + std::vector result; + for (const util::SettingsValue &value : + util::GetSettingsList(m_settings, m_network, SettingName(strArg), + ignore_default_section_config)) { + result.push_back( + value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str()); } - return result; } bool ArgsManager::IsArgSet(const std::string &strArg) const { - // special case - if (IsArgNegated(strArg)) { - return true; - } - return ArgsManagerHelper::GetArg(*this, strArg).first; + return !ArgsManagerHelper::Get(*this, strArg).isNull(); } bool ArgsManager::IsArgNegated(const std::string &strArg) const { - LOCK(cs_args); - - const auto &ov = m_override_args.find(strArg); - if (ov != m_override_args.end()) { - return ov->second.empty(); - } - - if (!m_network.empty()) { - const auto &cfs = - m_config_args.find(ArgsManagerHelper::NetworkArg(*this, strArg)); - if (cfs != m_config_args.end()) { - return cfs->second.empty(); - } - } - - const auto &cf = m_config_args.find(strArg); - if (cf != m_config_args.end()) { - return cf->second.empty(); - } - - return false; + return ArgsManagerHelper::Get(*this, strArg).isFalse(); } std::string ArgsManager::GetArg(const std::string &strArg, const std::string &strDefault) const { - if (IsArgNegated(strArg)) { - return "0"; - } - std::pair found_res = - ArgsManagerHelper::GetArg(*this, strArg); - if (found_res.first) { - return found_res.second; - } - return strDefault; + const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + return value.isNull() + ? strDefault + : value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str(); } int64_t ArgsManager::GetArg(const std::string &strArg, int64_t nDefault) const { - if (IsArgNegated(strArg)) { - return 0; - } - std::pair found_res = - ArgsManagerHelper::GetArg(*this, strArg); - if (found_res.first) { - return atoi64(found_res.second); - } - return nDefault; + const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + return value.isNull() + ? nDefault + : value.isFalse() + ? 0 + : value.isTrue() ? 1 + : value.isNum() ? value.get_int64() + : atoi64(value.get_str()); } bool ArgsManager::GetBoolArg(const std::string &strArg, bool fDefault) const { - if (IsArgNegated(strArg)) { - return false; - } - std::pair found_res = - ArgsManagerHelper::GetArg(*this, strArg); - if (found_res.first) { - return InterpretBool(found_res.second); - } - return fDefault; + const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + return value.isNull() ? fDefault + : value.isBool() ? value.get_bool() + : InterpretBool(value.get_str()); } bool ArgsManager::SoftSetArg(const std::string &strArg, @@ -617,7 +473,7 @@ void ArgsManager::ForceSetArg(const std::string &strArg, const std::string &strValue) { LOCK(cs_args); - m_override_args[strArg] = {strValue}; + m_settings.forced_settings[SettingName(strArg)] = strValue; } /** @@ -626,9 +482,15 @@ * integrity of mapMultiArgs data structure */ void ArgsManager::ForceSetMultiArg(const std::string &strArg, - const std::string &strValue) { + const std::vector &values) { LOCK(cs_args); - m_override_args[strArg].push_back(strValue); + util::SettingsValue value; + value.setArray(); + for (const std::string &s : values) { + value.push_back(s); + } + + m_settings.forced_settings[SettingName(strArg)] = value; } void ArgsManager::AddArg(const std::string &name, const std::string &help, @@ -661,7 +523,7 @@ void ArgsManager::ClearForcedArg(const std::string &strArg) { LOCK(cs_args); - m_override_args.erase(strArg); + m_settings.forced_settings.erase(SettingName(strArg)); } std::string ArgsManager::GetHelpMessage() const { @@ -977,13 +839,16 @@ return false; } for (const std::pair &option : options) { - const std::string strKey = std::string("-") + option.first; - const unsigned int flags = FlagsOfKnownArg(strKey); + std::string section; + std::string key = option.first; + util::SettingsValue value = + InterpretOption(section, key, option.second); + const unsigned int flags = FlagsOfKnownArg(key); if (flags) { - if (!InterpretOption(strKey, option.second, flags, m_config_args, - error)) { + if (!CheckValid(key, value, flags, error)) { return false; } + m_settings.ro_config[section][key].push_back(value); } else { if (ignore_invalid_keys) { LogPrintf("Ignoring unknown configuration value %s\n", @@ -1002,7 +867,7 @@ bool ignore_invalid_keys) { { LOCK(cs_args); - m_config_args.clear(); + m_settings.ro_config.clear(); m_config_sections.clear(); } @@ -1020,36 +885,40 @@ bool use_conf_file{true}; { LOCK(cs_args); - auto it = m_override_args.find("-includeconf"); - if (it != m_override_args.end()) { + if (auto *includes = util::FindKey(m_settings.command_line_options, + "includeconf")) { // ParseParameters() fails if a non-negated -includeconf is // passed on the command-line - assert(it->second.empty()); + assert(util::SettingsSpan(*includes).last_negated()); use_conf_file = false; } } if (use_conf_file) { std::string chain_id = GetChainName(); - std::vector conf_file_names(GetArgs("-includeconf")); - { - // We haven't set m_network yet (that happens in - // SelectParams()), so manually check for network.includeconf - // args. - std::vector includeconf_net( - GetArgs(std::string("-") + chain_id + ".includeconf")); - conf_file_names.insert(conf_file_names.end(), - includeconf_net.begin(), - includeconf_net.end()); - } + std::vector conf_file_names; - // Remove -includeconf from configuration, so we can warn about - // recursion later - { + auto add_includes = [&](const std::string &network, + size_t skip = 0) { + size_t num_values = 0; LOCK(cs_args); - m_config_args.erase("-includeconf"); - m_config_args.erase(std::string("-") + chain_id + - ".includeconf"); - } + if (auto *section = + util::FindKey(m_settings.ro_config, network)) { + if (auto *values = util::FindKey(*section, "includeconf")) { + for (size_t i = std::max( + skip, util::SettingsSpan(*values).negated()); + i < values->size(); ++i) { + conf_file_names.push_back((*values)[i].get_str()); + } + num_values = values->size(); + } + } + return num_values; + }; + + // We haven't set m_network yet (that happens in SelectParams()), so + // manually check for network.includeconf args. + const size_t chain_includes = add_includes(chain_id); + const size_t default_includes = add_includes({}); for (const std::string &conf_file_name : conf_file_names) { fsbridge::ifstream conf_file_stream( @@ -1069,21 +938,14 @@ } // Warn about recursive -includeconf - conf_file_names = GetArgs("-includeconf"); - std::vector includeconf_net( - GetArgs(std::string("-") + chain_id + ".includeconf")); - conf_file_names.insert(conf_file_names.end(), - includeconf_net.begin(), - includeconf_net.end()); + conf_file_names.clear(); + add_includes(chain_id, /* skip= */ chain_includes); + add_includes({}, /* skip= */ default_includes); std::string chain_id_final = GetChainName(); if (chain_id_final != chain_id) { // Also warn about recursive includeconf for the chain that was // specified in one of the includeconfs - includeconf_net = - GetArgs(std::string("-") + chain_id_final + ".includeconf"); - conf_file_names.insert(conf_file_names.end(), - includeconf_net.begin(), - includeconf_net.end()); + add_includes(chain_id_final); } for (const std::string &conf_file_name : conf_file_names) { tfm::format(std::cerr, @@ -1105,9 +967,19 @@ } std::string ArgsManager::GetChainName() const { - LOCK(cs_args); - const bool fRegTest = ArgsManagerHelper::GetNetBoolArg(*this, "-regtest"); - const bool fTestNet = ArgsManagerHelper::GetNetBoolArg(*this, "-testnet"); + auto get_net = [&](const std::string &arg) { + LOCK(cs_args); + util::SettingsValue value = + GetSetting(m_settings, /* section= */ "", SettingName(arg), + /* ignore_default_section_config= */ false, + /* get_chain_name= */ true); + return value.isNull() ? false + : value.isBool() ? value.get_bool() + : InterpretBool(value.get_str()); + }; + + const bool fRegTest = get_net("-regtest"); + const bool fTestNet = get_net("-testnet"); const bool is_chain_arg_set = IsArgSet("-chain"); if (int(is_chain_arg_set) + int(fRegTest) + int(fTestNet) > 1) { diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -26,6 +26,7 @@ # list false positive unknows arguments SET_FALSE_POSITIVE_UNKNOWNS = set([ + '-includeconf', '-regtest', '-testnet', '-zmqpubhashblock',