From 20fb7618b001937befbf8757e5881674b4a55147 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Fri, 6 Mar 2026 11:03:57 -0800 Subject: [PATCH] args: make most ArgsManager members private Move the first `protected` block (struct Arg, cs_args, m_settings, and all other member variables) to `private`. Only `ReadConfigStream` and `ReadConfigString` remain `protected` for test access. Changes: - Move `ReadConfigString` from `TestArgsManager` into `ArgsManager` itself (declared in args.h, defined in config.cpp) so tests no longer need direct access to `cs_args` or `m_settings` for config parsing. - Replace test-only `SetNetworkOnlyArg` helper with the existing `NETWORK_ONLY` flag passed through `SetupArgs`/`AddArg`. - Remove `TestArgsManager` constructor that cleared `m_network_only_args`. - Remove `using` declarations for `cs_args`, `m_settings`, `GetSetting`, and `GetSettingsList` from `TestArgsManager`. - Clear `m_config_sections` in `ClearArgs()`. Co-authored-by: Anthony Towns --- src/common/args.cpp | 1 + src/common/args.h | 10 ++++++---- src/common/config.cpp | 13 +++++++++++++ src/test/argsman_tests.cpp | 35 ++++++++++++----------------------- 4 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/common/args.cpp b/src/common/args.cpp index be8c513339c..2a9a3c19fa5 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 fa22c36db7f..de323d3c490 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 85bea6740b6..a05c20d6f8f 100644 --- a/src/common/config.cpp +++ b/src/common/config.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -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 3500bc5cc96..c8802d3641c 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>& 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 argv = {"ignored"};