From 26a50ab322614bceb5bc62e2c282f83e5987bad8 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Sat, 21 Aug 2021 14:06:49 -0400 Subject: refactor: Split InterpretOption into Interpret{Key,Value} functions Co-authored-by: Anthony Towns --- src/util/system.cpp | 101 +++++++++++++++++++++++++++------------------------- 1 file changed, 52 insertions(+), 49 deletions(-) (limited to 'src/util/system.cpp') diff --git a/src/util/system.cpp b/src/util/system.cpp index 12d7dc49b2..2bc6b3bdf8 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -71,6 +71,7 @@ #endif #include +#include #include #include #include @@ -182,60 +183,65 @@ static std::string SettingName(const std::string& arg) return arg.size() > 0 && arg[0] == '-' ? arg.substr(1) : arg; } +struct KeyInfo { + std::string name; + std::string section; + bool negated{false}; +}; + /** - * Interpret -nofoo as if the user supplied -foo=0. - * - * 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 false. + * Parse "name", "section.name", "noname", "section.noname" settings keys. * - * If there was a double negative, it removes "no" from the key, and - * returns true. - * - * If there was no "no", it returns the string value untouched. - * - * Where an option was negated can be later checked using the + * @note 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). */ - -static util::SettingsValue InterpretOption(std::string& section, std::string& key, const std::string& value) +KeyInfo InterpretKey(std::string key) { + KeyInfo result; // 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) { - section = key.substr(0, option_index); + result.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; + result.negated = true; } - return value; + result.name = key; + return result; } /** - * Check settings value validity according to flags. + * Interpret settings value based on registered flags. + * + * @param[in] key key information to know if key was negated + * @param[in] value string value of setting to be parsed + * @param[in] flags ArgsManager registered argument flags + * @param[out] error Error description if settings value is not valid * - * 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 + * @return parsed settings value if it is valid, otherwise nullopt accompanied + * by a descriptive error string */ -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); +static std::optional InterpretValue(const KeyInfo& key, const std::string& value, + unsigned int flags, std::string& error) +{ + // Return negated settings as false values. + if (key.negated) { + if (!(flags & ArgsManager::ALLOW_BOOL)) { + error = strprintf("Negating of -%s is meaningless and therefore forbidden", key.name); + return std::nullopt; + } + // Double negatives like -nofoo=0 are supported (but discouraged) + if (!InterpretBool(value)) { + LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key.name, value); + return true; + } return false; } - return true; + return value; } namespace { @@ -351,21 +357,21 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin // Transform -foo to foo key.erase(0, 1); - std::string section; - util::SettingsValue value = InterpretOption(section, key, val); - std::optional flags = GetArgFlags('-' + key); + KeyInfo keyinfo = InterpretKey(key); + std::optional flags = GetArgFlags('-' + keyinfo.name); // Unknown command line options and command line options with dot - // characters (which are returned from InterpretOption with nonempty + // characters (which are returned from InterpretKey with nonempty // section strings) are not valid. - if (!flags || !section.empty()) { + if (!flags || !keyinfo.section.empty()) { error = strprintf("Invalid parameter %s", argv[i]); return false; } - if (!CheckValid(key, value, *flags, error)) return false; + std::optional value = InterpretValue(keyinfo, val, *flags, error); + if (!value) return false; - m_settings.command_line_options[key].push_back(value); + m_settings.command_line_options[keyinfo.name].push_back(*value); } // we do not allow -includeconf from command line, only -noincludeconf @@ -548,10 +554,8 @@ bool ArgsManager::ReadSettingsFile(std::vector* errors) return false; } for (const auto& setting : m_settings.rw_settings) { - std::string section; - std::string key = setting.first; - (void)InterpretOption(section, key, /* value */ {}); // Split setting key into section and argname - if (!GetArgFlags('-' + key)) { + KeyInfo key = InterpretKey(setting.first); // Split setting key into section and argname + if (!GetArgFlags('-' + key.name)) { LogPrintf("Ignoring unknown rw_settings value %s\n", setting.first); } } @@ -870,15 +874,14 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file return false; } for (const std::pair& option : options) { - std::string section; - std::string key = option.first; - util::SettingsValue value = InterpretOption(section, key, option.second); - std::optional flags = GetArgFlags('-' + key); + KeyInfo key = InterpretKey(option.first); + std::optional flags = GetArgFlags('-' + key.name); if (flags) { - if (!CheckValid(key, value, *flags, error)) { + std::optional value = InterpretValue(key, option.second, *flags, error); + if (!value) { return false; } - m_settings.ro_config[section][key].push_back(value); + m_settings.ro_config[key.section][key.name].push_back(*value); } else { if (ignore_invalid_keys) { LogPrintf("Ignoring unknown configuration value %s\n", option.first); -- cgit v1.2.3