Skip to content

Commit 4d34fcc

Browse files
ajtownsajtowns
committed
ArgsManager: drop m_negated_args
When a -nofoo option is seen, instead of adding it to a separate set of negated args, set the arg as being an empty vector of strings. This changes the behaviour in some ways: - -nofoo=0 still sets foo=1 but no longer treats it as a negated arg - -nofoo=1 -foo=2 has GetArgs() return [2] rather than [2,0] - "foo=2 \n -nofoo=1" in a config file no longer returns [2,0], just [0] - GetArgs returns an empty vector for negated args
1 parent 3673ca3 commit 4d34fcc

File tree

3 files changed

+64
-54
lines changed

3 files changed

+64
-54
lines changed

‎src/test/util_tests.cpp

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,6 @@ struct TestArgsManager : public ArgsManager
189189
{
190190
std::map<std::string, std::vector<std::string> >& GetOverrideArgs() { return m_override_args; }
191191
std::map<std::string, std::vector<std::string> >& GetConfigArgs() { return m_config_args; }
192-
const std::unordered_set<std::string>& GetNegatedArgs() { return m_negated_args; }
193192
void ReadConfigString(const std::string str_config)
194193
{
195194
std::istringstream streamConfig(str_config);
@@ -250,7 +249,6 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
250249

251250
// The -b option is flagged as negated, and nothing else is
252251
BOOST_CHECK(testArgs.IsArgNegated("-b"));
253-
BOOST_CHECK(testArgs.GetNegatedArgs().size() == 1);
254252
BOOST_CHECK(!testArgs.IsArgNegated("-a"));
255253

256254
// Check expected values.
@@ -275,8 +273,8 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
275273
BOOST_CHECK(!testArgs.IsArgNegated("-foo"));
276274
BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "");
277275

278-
// A double negative is a positive.
279-
BOOST_CHECK(testArgs.IsArgNegated("-bar"));
276+
// A double negative is a positive, and not marked as negated.
277+
BOOST_CHECK(!testArgs.IsArgNegated("-bar"));
280278
BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "1");
281279

282280
// Config test
@@ -285,12 +283,12 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
285283
testArgs.ReadConfigString(conf_test);
286284

287285
// This was passed twice, second one overrides the negative setting,
288-
// but not the value.
286+
// and the value.
289287
BOOST_CHECK(!testArgs.IsArgNegated("-foo"));
290-
BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "0");
288+
BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "1");
291289

292-
// A double negative is a positive.
293-
BOOST_CHECK(testArgs.IsArgNegated("-bar"));
290+
// A double negative is a positive, and does not count as negated.
291+
BOOST_CHECK(!testArgs.IsArgNegated("-bar"));
294292
BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "1");
295293

296294
// Combined test
@@ -300,18 +298,15 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases)
300298
testArgs.ReadConfigString(combo_test_conf);
301299

302300
// Command line overrides, but doesn't erase old setting
303-
BOOST_CHECK(!testArgs.IsArgNegated("-foo"));
301+
BOOST_CHECK(testArgs.IsArgNegated("-foo"));
304302
BOOST_CHECK(testArgs.GetArg("-foo", "xxx") == "0");
305-
BOOST_CHECK(testArgs.GetArgs("-foo").size() == 2
306-
&& testArgs.GetArgs("-foo").front() == "0"
307-
&& testArgs.GetArgs("-foo").back() == "1");
303+
BOOST_CHECK(testArgs.GetArgs("-foo").size() == 0);
308304

309305
// Command line overrides, but doesn't erase old setting
310-
BOOST_CHECK(testArgs.IsArgNegated("-bar"));
306+
BOOST_CHECK(!testArgs.IsArgNegated("-bar"));
311307
BOOST_CHECK(testArgs.GetArg("-bar", "xxx") == "");
312-
BOOST_CHECK(testArgs.GetArgs("-bar").size() == 2
313-
&& testArgs.GetArgs("-bar").front() == ""
314-
&& testArgs.GetArgs("-bar").back() == "0");
308+
BOOST_CHECK(testArgs.GetArgs("-bar").size() == 1
309+
&& testArgs.GetArgs("-bar").front() == "");
315310
}
316311

317312
BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
@@ -364,8 +359,8 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
364359
&& test_args.GetArg("-d", "xxx") == "e"
365360
&& test_args.GetArg("-fff", "xxx") == "0"
366361
&& test_args.GetArg("-ggg", "xxx") == "1"
367-
&& test_args.GetArg("-h", "xxx") == "1" // 1st value takes precedence
368-
&& test_args.GetArg("-i", "xxx") == "0" // 1st value takes precedence
362+
&& test_args.GetArg("-h", "xxx") == "0"
363+
&& test_args.GetArg("-i", "xxx") == "1"
369364
&& test_args.GetArg("-zzz", "xxx") == "xxx"
370365
);
371366

@@ -376,8 +371,8 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
376371
&& !test_args.GetBoolArg("-d", def)
377372
&& !test_args.GetBoolArg("-fff", def)
378373
&& test_args.GetBoolArg("-ggg", def)
379-
&& test_args.GetBoolArg("-h", def)
380-
&& !test_args.GetBoolArg("-i", def)
374+
&& !test_args.GetBoolArg("-h", def)
375+
&& test_args.GetBoolArg("-i", def)
381376
&& test_args.GetBoolArg("-zzz", def) == def
382377
);
383378
}
@@ -389,19 +384,15 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
389384
BOOST_CHECK(test_args.GetArgs("-ccc").size() == 2
390385
&& test_args.GetArgs("-ccc").front() == "argument"
391386
&& test_args.GetArgs("-ccc").back() == "multiple");
392-
BOOST_CHECK(test_args.GetArgs("-fff").size() == 1
393-
&& test_args.GetArgs("-fff").front() == "0");
387+
BOOST_CHECK(test_args.GetArgs("-fff").size() == 0);
394388
BOOST_CHECK(test_args.GetArgs("-nofff").size() == 0);
395389
BOOST_CHECK(test_args.GetArgs("-ggg").size() == 1
396390
&& test_args.GetArgs("-ggg").front() == "1");
397391
BOOST_CHECK(test_args.GetArgs("-noggg").size() == 0);
398-
BOOST_CHECK(test_args.GetArgs("-h").size() == 2
399-
&& test_args.GetArgs("-h").front() == "1"
400-
&& test_args.GetArgs("-h").back() == "0");
392+
BOOST_CHECK(test_args.GetArgs("-h").size() == 0);
401393
BOOST_CHECK(test_args.GetArgs("-noh").size() == 0);
402-
BOOST_CHECK(test_args.GetArgs("-i").size() == 2
403-
&& test_args.GetArgs("-i").front() == "0"
404-
&& test_args.GetArgs("-i").back() == "1");
394+
BOOST_CHECK(test_args.GetArgs("-i").size() == 1
395+
&& test_args.GetArgs("-i").front() == "1");
405396
BOOST_CHECK(test_args.GetArgs("-noi").size() == 0);
406397
BOOST_CHECK(test_args.GetArgs("-zzz").size() == 0);
407398

@@ -410,7 +401,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
410401
BOOST_CHECK(!test_args.IsArgNegated("-ccc"));
411402
BOOST_CHECK(!test_args.IsArgNegated("-d"));
412403
BOOST_CHECK(test_args.IsArgNegated("-fff"));
413-
BOOST_CHECK(test_args.IsArgNegated("-ggg")); // IsArgNegated==true when noggg=0
404+
BOOST_CHECK(!test_args.IsArgNegated("-ggg"));
414405
BOOST_CHECK(test_args.IsArgNegated("-h")); // last setting takes precedence
415406
BOOST_CHECK(!test_args.IsArgNegated("-i")); // last setting takes precedence
416407
BOOST_CHECK(!test_args.IsArgNegated("-zzz"));

‎src/util.cpp

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -519,35 +519,44 @@ class ArgsManagerHelper {
519519
/**
520520
* Interpret -nofoo as if the user supplied -foo=0.
521521
*
522-
* This method also tracks when the -no form was supplied, and treats "-foo" as
523-
* a negated option when this happens. This can be later checked using the
522+
* This method also tracks when the -no form was supplied, and if so,
523+
* checks whether there was a double-negative (-nofoo=0 -> -foo=1).
524+
*
525+
* If there was not a double negative, it removes the "no" from the key,
526+
* and returns true, indicating the caller should clear the args vector
527+
* to indicate a negated option.
528+
*
529+
* If there was a double negative, it removes "no" from the key, sets the
530+
* value to "1" and returns false.
531+
*
532+
* If there was no "no", it leaves key and value untouched and returns
533+
* false.
534+
*
535+
* Where an option was negated can be later checked using the
524536
* IsArgNegated() method. One use case for this is to have a way to disable
525537
* options that are not normally boolean (e.g. using -nodebuglogfile to request
526538
* that debug log output is not sent to any file at all).
527539
*/
528-
void ArgsManager::InterpretNegatedOption(std::string& key, std::string& val)
540+
static bool InterpretNegatedOption(std::string& key, std::string& val)
529541
{
530542
if (key.substr(0, 3) == "-no") {
531543
bool bool_val = InterpretBool(val);
544+
key.erase(1, 2);
532545
if (!bool_val ) {
533546
// Double negatives like -nofoo=0 are supported (but discouraged)
534547
LogPrintf("Warning: parsed potentially confusing double-negative %s=%s\n", key, val);
548+
val = "1";
549+
} else {
550+
return true;
535551
}
536-
key.erase(1, 2);
537-
m_negated_args.insert(key);
538-
val = bool_val ? "0" : "1";
539-
} else {
540-
// In an invocation like "bitcoind -nofoo -foo" we want to unmark -foo
541-
// as negated when we see the second option.
542-
m_negated_args.erase(key);
543552
}
553+
return false;
544554
}
545555

546556
void ArgsManager::ParseParameters(int argc, const char* const argv[])
547557
{
548558
LOCK(cs_args);
549559
m_override_args.clear();
550-
m_negated_args.clear();
551560

552561
for (int i = 1; i < argc; i++) {
553562
std::string key(argv[i]);
@@ -570,16 +579,19 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
570579
if (key.length() > 1 && key[1] == '-')
571580
key.erase(0, 1);
572581

573-
// Transform -nofoo to -foo=0
574-
InterpretNegatedOption(key, val);
575-
576-
m_override_args[key].push_back(val);
582+
// Check for -nofoo
583+
if (InterpretNegatedOption(key, val)) {
584+
m_override_args[key].clear();
585+
} else {
586+
m_override_args[key].push_back(val);
587+
}
577588
}
578589
}
579590

580591
std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
581592
{
582593
std::vector<std::string> result = {};
594+
if (IsArgNegated(strArg)) return result; // special case
583595

584596
LOCK(cs_args);
585597
ArgsManagerHelper::AddArgs(result, m_override_args, strArg);
@@ -589,31 +601,42 @@ std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
589601

590602
bool ArgsManager::IsArgSet(const std::string& strArg) const
591603
{
604+
if (IsArgNegated(strArg)) return true; // special case
592605
return ArgsManagerHelper::GetArg(*this, strArg).first;
593606
}
594607

595608
bool ArgsManager::IsArgNegated(const std::string& strArg) const
596609
{
597610
LOCK(cs_args);
598-
return m_negated_args.find(strArg) != m_negated_args.end();
611+
612+
const auto& ov = m_override_args.find(strArg);
613+
if (ov != m_override_args.end()) return ov->second.empty();
614+
615+
const auto& cf = m_config_args.find(strArg);
616+
if (cf != m_config_args.end()) return cf->second.empty();
617+
618+
return false;
599619
}
600620

601621
std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const
602622
{
623+
if (IsArgNegated(strArg)) return "0";
603624
std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
604625
if (found_res.first) return found_res.second;
605626
return strDefault;
606627
}
607628

608629
int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const
609630
{
631+
if (IsArgNegated(strArg)) return 0;
610632
std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
611633
if (found_res.first) return atoi64(found_res.second);
612634
return nDefault;
613635
}
614636

615637
bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const
616638
{
639+
if (IsArgNegated(strArg)) return false;
617640
std::pair<bool,std::string> found_res = ArgsManagerHelper::GetArg(*this, strArg);
618641
if (found_res.first) return InterpretBool(found_res.second);
619642
return fDefault;
@@ -801,11 +824,13 @@ void ArgsManager::ReadConfigStream(std::istream& stream)
801824

802825
for (boost::program_options::detail::config_file_iterator it(stream, setOptions), end; it != end; ++it)
803826
{
804-
// Don't overwrite existing settings so command line settings override bitcoin.conf
805827
std::string strKey = std::string("-") + it->string_key;
806828
std::string strValue = it->value[0];
807-
InterpretNegatedOption(strKey, strValue);
808-
m_config_args[strKey].push_back(strValue);
829+
if (InterpretNegatedOption(strKey, strValue)) {
830+
m_config_args[strKey].clear();
831+
} else {
832+
m_config_args[strKey].push_back(strValue);
833+
}
809834
}
810835
}
811836

‎src/util.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,6 @@ class ArgsManager
228228
mutable CCriticalSection cs_args;
229229
std::map<std::string, std::vector<std::string>> m_override_args;
230230
std::map<std::string, std::vector<std::string>> m_config_args;
231-
std::unordered_set<std::string> m_negated_args;
232231
void ReadConfigStream(std::istream& stream);
233232

234233
public:
@@ -314,11 +313,6 @@ class ArgsManager
314313
* @return CBaseChainParams::MAIN by default; raises runtime error if an invalid combination is given.
315314
*/
316315
std::string GetChainName() const;
317-
318-
private:
319-
320-
// Munge -nofoo into -foo=0 and track the value as negated.
321-
void InterpretNegatedOption(std::string &key, std::string &val);
322316
};
323317

324318
extern ArgsManager gArgs;

0 commit comments

Comments
 (0)