args: make most ArgsManager members private
What changed, and why it matters
This is a code cleanup change that hides internal details of Bitcoin Core's command-line argument manager and improves how tests access it. It does not fix a security bug or change user-visible behavior.
No security action needed; treat as ordinary refactoring/test-hygiene review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors ArgsManager so most protected member variables become private, moves ReadConfigString into ArgsManager itself, removes test-only helpers that reached into internal state, and makes tests use the public NETWORK_ONLY flag instead of directly mutating m_network_only_args. It also clears m_config_sections in ClearArgs(). There is no vulnerability fix.
Changed components
src/common/args.hsrc/common/args.cppsrc/common/config.cppsrc/test/argsman_tests.cppInspect captured patch +32 / −27
diff --git a/src/common/args.cpp b/src/common/args.cpp
index be8c5133..2a9a3c19 100644
--- a/src/common/args.cpp
+++ b/src/common/args.cpp
@@ -644,6 +644,7 @@ void ArgsManager::ClearArgs()
m_settings = {};
m_available_args.clear();
m_network_only_args.clear();
+ m_config_sections.clear();
}
void ArgsManager::CheckMultipleCLIArgs() const
diff --git a/src/common/args.h b/src/common/args.h
index fa22c36d..de323d3c 100644
--- a/src/common/args.h
+++ b/src/common/args.h
@@ -127,7 +127,7 @@ public:
COMMAND = 0x800,
};
-protected:
+private:
struct Arg
{
std::string m_help_param;
@@ -149,8 +149,6 @@ protected:
mutable fs::path m_cached_datadir_path GUARDED_BY(cs_args);
mutable fs::path m_cached_network_datadir_path GUARDED_BY(cs_args);
- [[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
-
/**
* Returns true if settings values from the default section should be used,
* depending on the current network and whether the setting is
@@ -158,7 +156,11 @@ protected:
*/
bool UseDefaultSection(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
- public:
+protected:
+ [[nodiscard]] bool ReadConfigStream(std::istream& stream, const std::string& filepath, std::string& error, bool ignore_invalid_keys = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+ [[nodiscard]] bool ReadConfigString(const std::string& str_config) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
+
+public:
/**
* Get setting value.
*
diff --git a/src/common/config.cpp b/src/common/config.cpp
index 85bea674..a05c20d6 100644
--- a/src/common/config.cpp
+++ b/src/common/config.cpp
@@ -19,6 +19,7 @@
#include <filesystem>
#include <fstream>
#include <iostream>
+#include <sstream>
#include <list>
#include <map>
#include <memory>
@@ -119,6 +120,18 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file
return true;
}
+bool ArgsManager::ReadConfigString(const std::string& str_config)
+{
+ std::istringstream streamConfig(str_config);
+ {
+ LOCK(cs_args);
+ m_settings.ro_config.clear();
+ m_config_sections.clear();
+ }
+ std::string error;
+ return ReadConfigStream(streamConfig, "", error);
+}
+
bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
{
{
diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp
index 3500bc5c..c8802d36 100644
--- a/src/test/argsman_tests.cpp
+++ b/src/test/argsman_tests.cpp
@@ -51,22 +51,9 @@ BOOST_AUTO_TEST_CASE(util_datadir)
struct TestArgsManager : public ArgsManager
{
- TestArgsManager() { m_network_only_args.clear(); }
void ReadConfigString(const std::string& str_config)
{
- std::istringstream streamConfig(str_config);
- {
- LOCK(cs_args);
- m_settings.ro_config.clear();
- m_config_sections.clear();
- }
- std::string error;
- BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error));
- }
- void SetNetworkOnlyArg(const std::string& arg)
- {
- LOCK(cs_args);
- m_network_only_args.insert(arg);
+ BOOST_REQUIRE(ArgsManager::ReadConfigString(str_config));
}
void SetupArgs(const std::vector<std::pair<std::string, unsigned int>>& args)
{
@@ -74,11 +61,9 @@ struct TestArgsManager : public ArgsManager
AddArg(arg.first, "", arg.second, OptionsCategory::OPTIONS);
}
}
- using ArgsManager::GetSetting;
- using ArgsManager::GetSettingsList;
+
+ // make protected methods available for testing
using ArgsManager::ReadConfigStream;
- using ArgsManager::cs_args;
- using ArgsManager::m_settings;
};
//! Test GetSetting and GetArg type coercion, negation, and default value handling.
@@ -584,9 +569,12 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
// Test section only options
- test_args.SetNetworkOnlyArg("-d");
- test_args.SetNetworkOnlyArg("-ccc");
- test_args.SetNetworkOnlyArg("-h");
+ const auto ccc2 = std::make_pair("-ccc", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY);
+ const auto d2 = std::make_pair("-d", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY);
+ const auto h2 = std::make_pair("-h", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY);
+ test_args.ClearArgs();
+ test_args.SetupArgs({a, b, ccc2, d2, e, fff, ggg, h2, i, iii});
+ test_args.ReadConfigString(str_config);
test_args.SelectConfigNetwork(ChainTypeToString(ChainType::MAIN));
BOOST_CHECK(test_args.GetArg("-d", "xxx") == "e");
@@ -828,8 +816,9 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup)
const std::string& name = net_specific ? "wallet" : "server";
const std::string key = "-" + name;
- parser.AddArg(key, name, ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
- if (net_specific) parser.SetNetworkOnlyArg(key);
+ unsigned int flags = ArgsManager::ALLOW_ANY;
+ if (net_specific) flags |= ArgsManager::NETWORK_ONLY;
+ parser.AddArg(key, name, flags, OptionsCategory::OPTIONS);
auto args = GetValues(arg_actions, section, name, "a");
std::vector<const char*> argv = {"ignored"};
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.