Skip to content

Commit

Permalink
Fix argument parsing oddity with -noX
Browse files Browse the repository at this point in the history
`bitcoind -X -noX` ends up, unintuitively, with `X` set.
(for all boolean options X)

This result is due to the odd two-pass processing of arguments. This
patch fixes this oddity and simplifies the code at the same time.
  • Loading branch information
laanwj committed Jul 27, 2015
1 parent ca37e0f commit c38c49d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 39 deletions.
22 changes: 11 additions & 11 deletions src/test/getarg_tests.cpp
Expand Up @@ -60,18 +60,18 @@ BOOST_AUTO_TEST_CASE(boolarg)
BOOST_CHECK(!GetBoolArg("-foo", false));
BOOST_CHECK(!GetBoolArg("-foo", true));

ResetArgs("-foo -nofoo"); // -foo should win
BOOST_CHECK(GetBoolArg("-foo", false));
BOOST_CHECK(GetBoolArg("-foo", true));

ResetArgs("-foo=1 -nofoo=1"); // -foo should win
BOOST_CHECK(GetBoolArg("-foo", false));
BOOST_CHECK(GetBoolArg("-foo", true));
ResetArgs("-foo -nofoo"); // -nofoo should win
BOOST_CHECK(!GetBoolArg("-foo", false));
BOOST_CHECK(!GetBoolArg("-foo", true));

ResetArgs("-foo=0 -nofoo=0"); // -foo should win
ResetArgs("-foo=1 -nofoo=1"); // -nofoo should win
BOOST_CHECK(!GetBoolArg("-foo", false));
BOOST_CHECK(!GetBoolArg("-foo", true));

ResetArgs("-foo=0 -nofoo=0"); // -nofoo=0 should win
BOOST_CHECK(GetBoolArg("-foo", false));
BOOST_CHECK(GetBoolArg("-foo", true));

// New 0.6 feature: treat -- same as -:
ResetArgs("--foo=1");
BOOST_CHECK(GetBoolArg("-foo", false));
Expand Down Expand Up @@ -150,9 +150,9 @@ BOOST_AUTO_TEST_CASE(boolargno)
BOOST_CHECK(GetBoolArg("-foo", true));
BOOST_CHECK(GetBoolArg("-foo", false));

ResetArgs("-foo --nofoo");
BOOST_CHECK(GetBoolArg("-foo", true));
BOOST_CHECK(GetBoolArg("-foo", false));
ResetArgs("-foo --nofoo"); // --nofoo should win
BOOST_CHECK(!GetBoolArg("-foo", true));
BOOST_CHECK(!GetBoolArg("-foo", false));

ResetArgs("-nofoo -foo"); // foo always wins:
BOOST_CHECK(GetBoolArg("-foo", true));
Expand Down
47 changes: 19 additions & 28 deletions src/util.cpp
Expand Up @@ -315,18 +315,21 @@ int LogPrintStr(const std::string &str)
return ret;
}

static void InterpretNegativeSetting(string name, map<string, string>& mapSettingsRet)
/** Interpret string as boolean, for argument parsing */
static bool InterpretBool(const std::string& strValue)
{
// interpret -nofoo as -foo=0 (and -nofoo=0 as -foo=1) as long as -foo not set
if (name.find("-no") == 0)
if (strValue.empty())
return true;
return (atoi(strValue) != 0);
}

/** Turn -noX into -X=0 */
static void InterpretNegativeSetting(std::string& strKey, std::string& strValue)
{
if (strKey.length()>3 && strKey[0]=='-' && strKey[1]=='n' && strKey[2]=='o')
{
std::string positive("-");
positive.append(name.begin()+3, name.end());
if (mapSettingsRet.count(positive) == 0)
{
bool value = !GetBoolArg(name, false);
mapSettingsRet[positive] = (value ? "1" : "0");
}
strKey = "-" + strKey.substr(3);
strValue = InterpretBool(strValue) ? "0" : "1";
}
}

Expand Down Expand Up @@ -358,17 +361,11 @@ void ParseParameters(int argc, const char* const argv[])
// If both --foo and -foo are set, the last takes effect.
if (str.length() > 1 && str[1] == '-')
str = str.substr(1);
InterpretNegativeSetting(str, strValue);

mapArgs[str] = strValue;
mapMultiArgs[str].push_back(strValue);
}

// New 0.6 features:
BOOST_FOREACH(const PAIRTYPE(string,string)& entry, mapArgs)
{
// interpret -nofoo as -foo=0 (and -nofoo=0 as -foo=1) as long as -foo not set
InterpretNegativeSetting(entry.first, mapArgs);
}
}

std::string GetArg(const std::string& strArg, const std::string& strDefault)
Expand All @@ -388,11 +385,7 @@ int64_t GetArg(const std::string& strArg, int64_t nDefault)
bool GetBoolArg(const std::string& strArg, bool fDefault)
{
if (mapArgs.count(strArg))
{
if (mapArgs[strArg].empty())
return true;
return (atoi(mapArgs[strArg]) != 0);
}
return InterpretBool(mapArgs[strArg]);
return fDefault;
}

Expand Down Expand Up @@ -543,13 +536,11 @@ void ReadConfigFile(map<string, string>& mapSettingsRet,
{
// Don't overwrite existing settings so command line settings override bitcoin.conf
string strKey = string("-") + it->string_key;
string strValue = it->value[0];
InterpretNegativeSetting(strKey, strValue);
if (mapSettingsRet.count(strKey) == 0)
{
mapSettingsRet[strKey] = it->value[0];
// interpret nofoo=1 as foo=0 (and nofoo=0 as foo=1) as long as foo not set)
InterpretNegativeSetting(strKey, mapSettingsRet);
}
mapMultiSettingsRet[strKey].push_back(it->value[0]);
mapSettingsRet[strKey] = strValue;
mapMultiSettingsRet[strKey].push_back(strValue);
}
// If datadir is changed in .conf file:
ClearDatadirCache();
Expand Down

0 comments on commit c38c49d

Please sign in to comment.