Skip to content

Commit

Permalink
ArgsManager: keep command line and config file arguments separate
Browse files Browse the repository at this point in the history 
>>> backports bitcoin/bitcoin@3673ca3
  • Loading branch information
@random-zebra
random-zebra committed Apr 19, 2021
1 parent 4f416b8 commit 4ee69b5
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 57 deletions.
84 changes: 52 additions & 32 deletions src/test/util_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,17 @@ BOOST_AUTO_TEST_CASE(util_DateTimeStrFormat)

struct TestArgsManager : public ArgsManager
{
std::map<std::string, std::string>& GetMapArgs() { return mapArgs; }
const std::map<std::string, std::vector<std::string> >& GetMapMultiArgs() { return mapMultiArgs; }
std::map<std::string, std::vector<std::string> >& GetOverrideArgs() { return m_override_args; }
std::map<std::string, std::vector<std::string> >& GetConfigArgs() { return m_config_args; }
const std::unordered_set<std::string>& 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);
}
};

Expand All @@ -112,23 +116,26 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-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.GetArgs("-ccc").size() == 2);
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_AUTO_TEST_CASE(util_GetBoolArg)
Expand All @@ -143,8 +150,8 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
BOOST_CHECK(testArgs.IsArgSet({'-', opt}) || !opt);

// 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"));
Expand Down Expand Up @@ -235,17 +242,17 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
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.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.GetOverrideArgs().empty());
BOOST_CHECK(test_args.GetConfigArgs().size() == 8);

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")
Expand Down Expand Up @@ -320,16 +327,24 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
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");
Expand All @@ -340,6 +355,11 @@ BOOST_AUTO_TEST_CASE(util_GetArg)
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_GetChainName)
Expand Down
105 changes: 82 additions & 23 deletions src/util.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,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.
*
Expand Down Expand Up @@ -152,8 +213,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++) {
Expand All @@ -180,23 +240,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
Expand All @@ -207,25 +267,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;
}

Expand All @@ -248,8 +305,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};
}

static const int screenWidth = 79;
Expand Down Expand Up @@ -512,14 +568,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
Expand Down
6 changes: 4 additions & 2 deletions src/util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,11 @@ inline bool IsSwitchChar(char c)
class ArgsManager
{
protected:
friend class ArgsManagerHelper;

mutable RecursiveMutex cs_args;
std::map<std::string, std::string> mapArgs;
std::map<std::string, std::vector<std::string>> mapMultiArgs;
std::map<std::string, std::vector<std::string>> m_override_args;
std::map<std::string, std::vector<std::string>> m_config_args;
std::unordered_set<std::string> m_negated_args;

void ReadConfigStream(std::istream& stream);
Expand Down

0 comments on commit 4ee69b5

Please sign in to comment.