Merge bitcoin/bitcoin#34745: refactor: replace ArgsManager::cs_args RecursiveMutex with Mutex

20fb7618b001937befbf8757e5881674b4a55147 args: make most ArgsManager members private (w0xlt)
22b40f34f33f5e5555d0b378f005516ecfb45e6c args: replace cs_args RecursiveMutex with Mutex (w0xlt)
3a16ec8582bee1dfc82857205ba1037c17a735cb test: scope cs_args locks to avoid recursive locking (w0xlt)
70b51fef7afd705d109718447097935e49cc0c25 args: eliminate all recursive locking of cs_args (w0xlt)
7d61e03c701215e2956353195dff4734210a2765 args: extract lock-requiring internal helpers (w0xlt)

Pull request description:

  Part of #19303.

  Replace `ArgsManager::cs_args` from `RecursiveMutex` to `Mutex`.

  The conversion follows the pattern established in prior `RecursiveMutex` removals (e.g. `CAddrMan` in #19238, `CBlockPolicyEstimator` in #22014): extract private lock-held helpers with trailing underscore naming (`GetSetting_()`, `GetArgFlags_()`, `GetPathArg_()`), then replace recursive calls in methods that already hold `cs_args` with those helpers.

ACKs for top commit:
  l0rinc:
    ACK 20fb7618b001937befbf8757e5881674b4a55147
  rkrux:
    Concept ACK 20fb7618b0
  sedited:
    Re-ACK 20fb7618b001937befbf8757e5881674b4a55147
  hebasto:
    ACK 20fb7618b001937befbf8757e5881674b4a55147, only rebased and suggested changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/34745#pullrequestreview-3916715262).

Tree-SHA512: 7ab4278737f00deaa3f3da75e08469f91e95aa31e916820d02af737c754751ae4f73c1c8650f120eeff142a134f9209cf581499696a7b88ffc83d296515e40f2
This commit is contained in:
merge-script 2026-03-13 09:22:05 +01:00
commit 5440280891
No known key found for this signature in database
GPG Key ID: 9B79B45691DB4173
4 changed files with 201 additions and 154 deletions

View File

@ -206,7 +206,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
if (key[0] != '-') {
if (!m_accept_any_command && m_command.empty()) {
// The first non-dash arg is a registered command
std::optional<unsigned int> flags = GetArgFlags(key);
std::optional<unsigned int> flags = GetArgFlags_(key);
if (!flags || !(*flags & ArgsManager::COMMAND)) {
error = strprintf("Invalid command '%s'", argv[i]);
return false;
@ -227,7 +227,7 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
// Transform -foo to foo
key.erase(0, 1);
KeyInfo keyinfo = InterpretKey(key);
std::optional<unsigned int> flags = GetArgFlags('-' + keyinfo.name);
std::optional<unsigned int> flags = GetArgFlags_('-' + keyinfo.name);
// Unknown command line options and command line options with dot
// characters (which are returned from InterpretKey with nonempty
@ -255,9 +255,9 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin
return true;
}
std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const
std::optional<unsigned int> ArgsManager::GetArgFlags_(const std::string& name) const
{
LOCK(cs_args);
AssertLockHeld(cs_args);
for (const auto& arg_map : m_available_args) {
const auto search = arg_map.second.find(name);
if (search != arg_map.second.end()) {
@ -267,22 +267,36 @@ std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) co
return m_default_flags;
}
std::optional<unsigned int> ArgsManager::GetArgFlags(const std::string& name) const
{
LOCK(cs_args);
return GetArgFlags_(name);
}
void ArgsManager::SetDefaultFlags(std::optional<unsigned int> flags)
{
LOCK(cs_args);
m_default_flags = flags;
}
fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) const
fs::path ArgsManager::GetPathArg_(std::string arg, const fs::path& default_value) const
{
if (IsArgNegated(arg)) return fs::path{};
std::string path_str = GetArg(arg, "");
AssertLockHeld(cs_args);
const auto value = GetSetting_(arg);
if (value.isFalse()) return {};
std::string path_str = SettingToString(value, "");
if (path_str.empty()) return default_value;
fs::path result = fs::PathFromString(path_str).lexically_normal();
// Remove trailing slash, if present.
return result.has_filename() ? result : result.parent_path();
}
fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) const
{
LOCK(cs_args);
return GetPathArg_(std::move(arg), default_value);
}
fs::path ArgsManager::GetBlocksDirPath() const
{
LOCK(cs_args);
@ -292,14 +306,14 @@ fs::path ArgsManager::GetBlocksDirPath() const
// this function
if (!path.empty()) return path;
if (IsArgSet("-blocksdir")) {
path = fs::absolute(GetPathArg("-blocksdir"));
if (!GetSetting_("-blocksdir").isNull()) {
path = fs::absolute(GetPathArg_("-blocksdir"));
if (!fs::is_directory(path)) {
path = "";
return path;
}
} else {
path = GetDataDirBase();
path = GetDataDir(/*net_specific=*/false);
}
path /= fs::PathFromString(BaseParams().DataDir());
@ -308,15 +322,25 @@ fs::path ArgsManager::GetBlocksDirPath() const
return path;
}
fs::path ArgsManager::GetDataDirBase() const {
LOCK(cs_args);
return GetDataDir(/*net_specific=*/false);
}
fs::path ArgsManager::GetDataDirNet() const {
LOCK(cs_args);
return GetDataDir(/*net_specific=*/true);
}
fs::path ArgsManager::GetDataDir(bool net_specific) const
{
LOCK(cs_args);
AssertLockHeld(cs_args);
fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path;
// Used cached path if available
if (!path.empty()) return path;
const fs::path datadir{GetPathArg("-datadir")};
const fs::path datadir{GetPathArg_("-datadir")};
if (!datadir.empty()) {
path = fs::absolute(datadir);
if (!fs::is_directory(path)) {
@ -419,7 +443,7 @@ bool ArgsManager::ReadSettingsFile(std::vector<std::string>* errors)
}
for (const auto& setting : m_settings.rw_settings) {
KeyInfo key = InterpretKey(setting.first); // Split setting key into section and argname
if (!GetArgFlags('-' + key.name)) {
if (!GetArgFlags_('-' + key.name)) {
LogWarning("Ignoring unknown rw_settings value %s", setting.first);
}
}
@ -555,8 +579,8 @@ INSTANTIATE_INT_TYPE(uint64_t);
bool ArgsManager::SoftSetArg(const std::string& strArg, const std::string& strValue)
{
LOCK(cs_args);
if (IsArgSet(strArg)) return false;
ForceSetArg(strArg, strValue);
if (!GetSetting_(strArg).isNull()) return false;
m_settings.forced_settings[SettingName(strArg)] = strValue;
return true;
}
@ -620,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
@ -629,7 +654,7 @@ void ArgsManager::CheckMultipleCLIArgs() const
auto cmds = m_available_args.find(OptionsCategory::CLI_COMMANDS);
if (cmds != m_available_args.end()) {
for (const auto& [cmd, argspec] : cmds->second) {
if (IsArgSet(cmd)) {
if (!GetSetting_(cmd).isNull()) {
found.push_back(cmd);
}
}
@ -853,15 +878,22 @@ std::variant<ChainType, std::string> ArgsManager::GetChainArg() const
bool ArgsManager::UseDefaultSection(const std::string& arg) const
{
AssertLockHeld(cs_args);
return m_network == ChainTypeToString(ChainType::MAIN) || !m_network_only_args.contains(arg);
}
common::SettingsValue ArgsManager::GetSetting_(const std::string& arg) const
{
AssertLockHeld(cs_args);
return common::GetSetting(
m_settings, m_network, SettingName(arg), !UseDefaultSection(arg),
/*ignore_nonpersistent=*/false, /*get_chain_type=*/false);
}
common::SettingsValue ArgsManager::GetSetting(const std::string& arg) const
{
LOCK(cs_args);
return common::GetSetting(
m_settings, m_network, SettingName(arg), !UseDefaultSection(arg),
/*ignore_nonpersistent=*/false, /*get_chain_type=*/false);
return GetSetting_(arg);
}
std::vector<common::SettingsValue> ArgsManager::GetSettingsList(const std::string& arg) const
@ -875,10 +907,11 @@ void ArgsManager::logArgsPrefix(
const std::string& section,
const std::map<std::string, std::vector<common::SettingsValue>>& args) const
{
AssertLockHeld(cs_args);
std::string section_str = section.empty() ? "" : "[" + section + "] ";
for (const auto& arg : args) {
for (const auto& value : arg.second) {
std::optional<unsigned int> flags = GetArgFlags('-' + arg.first);
std::optional<unsigned int> flags = GetArgFlags_('-' + arg.first);
if (flags) {
std::string value_str = (*flags & SENSITIVE) ? "****" : value.write();
LogInfo("%s %s%s=%s\n", prefix, section_str, arg.first, value_str);

View File

@ -127,7 +127,7 @@ public:
COMMAND = 0x800,
};
protected:
private:
struct Arg
{
std::string m_help_param;
@ -135,7 +135,7 @@ protected:
unsigned int m_flags;
};
mutable RecursiveMutex cs_args;
mutable Mutex cs_args;
common::Settings m_settings GUARDED_BY(cs_args);
std::vector<std::string> m_command GUARDED_BY(cs_args);
std::string m_network GUARDED_BY(cs_args);
@ -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);
/**
* 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.
*
@ -166,12 +168,12 @@ protected:
* false if "-nosetting" argument was passed, and a string if a "-setting=value"
* argument was passed.
*/
common::SettingsValue GetSetting(const std::string& arg) const;
common::SettingsValue GetSetting(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get list of setting values.
*/
std::vector<common::SettingsValue> GetSettingsList(const std::string& arg) const;
std::vector<common::SettingsValue> GetSettingsList(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
ArgsManager();
~ArgsManager();
@ -179,16 +181,16 @@ protected:
/**
* Select the network in use
*/
void SelectConfigNetwork(const std::string& network);
void SelectConfigNetwork(const std::string& network) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
[[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error);
[[nodiscard]] bool ParseParameters(int argc, const char* const argv[], std::string& error) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return config file path (read-only)
*/
fs::path GetConfigFilePath() const;
void SetConfigFilePath(fs::path);
[[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false);
fs::path GetConfigFilePath() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
void SetConfigFilePath(fs::path) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
[[nodiscard]] bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Log warnings for options in m_section_only_args when
@ -196,12 +198,12 @@ protected:
* on the command line or in a network-specific section in the
* config file.
*/
std::set<std::string> GetUnsuitableSectionOnlyArgs() const;
std::set<std::string> GetUnsuitableSectionOnlyArgs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Log warnings for unrecognized section names in the config file.
*/
std::list<SectionInfo> GetUnrecognizedSections() const;
std::list<SectionInfo> GetUnrecognizedSections() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
struct Command {
/** The command (if one has been registered with AddCommand), or empty */
@ -215,33 +217,33 @@ protected:
/**
* Get the command and command args (returns std::nullopt if no command provided)
*/
std::optional<const Command> GetCommand() const;
std::optional<const Command> GetCommand() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get blocks directory path
*
* @return Blocks path which is network specific
*/
fs::path GetBlocksDirPath() const;
fs::path GetBlocksDirPath() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get data directory path
*
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
fs::path GetDataDirBase() const { return GetDataDir(false); }
fs::path GetDataDirBase() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get data directory path with appended network identifier
*
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
fs::path GetDataDirNet() const { return GetDataDir(true); }
fs::path GetDataDirNet() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Clear cached directory paths
*/
void ClearPathCache();
void ClearPathCache() EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return a vector of strings of the given argument
@ -249,7 +251,7 @@ protected:
* @param strArg Argument to get (e.g. "-foo")
* @return command-line arguments
*/
std::vector<std::string> GetArgs(const std::string& strArg) const;
std::vector<std::string> GetArgs(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return true if the given argument has been manually set
@ -257,7 +259,7 @@ protected:
* @param strArg Argument to get (e.g. "-foo")
* @return true if the argument has been set
*/
bool IsArgSet(const std::string& strArg) const;
bool IsArgSet(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return true if the argument was originally passed as a negated option,
@ -266,7 +268,7 @@ protected:
* @param strArg Argument to get (e.g. "-foo")
* @return true if the argument was passed negated
*/
bool IsArgNegated(const std::string& strArg) const;
bool IsArgNegated(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return string argument or default value
@ -275,8 +277,8 @@ protected:
* @param strDefault (e.g. "1")
* @return command-line argument or default value
*/
std::string GetArg(const std::string& strArg, const std::string& strDefault) const;
std::optional<std::string> GetArg(const std::string& strArg) const;
std::string GetArg(const std::string& strArg, const std::string& strDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
std::optional<std::string> GetArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return path argument or default value
@ -288,7 +290,7 @@ protected:
* for examples or implementation for details). If argument is empty or not
* set, default_value is returned unchanged.
*/
fs::path GetPathArg(std::string arg, const fs::path& default_value = {}) const;
fs::path GetPathArg(std::string arg, const fs::path& default_value = {}) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return integer argument or default value
@ -298,13 +300,13 @@ protected:
* @return command-line argument (0 if invalid number) or default value
*/
template <std::integral Int>
Int GetArg(const std::string& strArg, Int nDefault) const;
Int GetArg(const std::string& strArg, Int nDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
template <std::integral Int>
std::optional<Int> GetArg(const std::string& strArg) const;
std::optional<Int> GetArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
int64_t GetIntArg(const std::string& strArg, int64_t nDefault) const { return GetArg<int64_t>(strArg, nDefault); }
std::optional<int64_t> GetIntArg(const std::string& strArg) const { return GetArg<int64_t>(strArg); }
int64_t GetIntArg(const std::string& strArg, int64_t nDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args) { return GetArg<int64_t>(strArg, nDefault); }
std::optional<int64_t> GetIntArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args) { return GetArg<int64_t>(strArg); }
/**
* Return boolean argument or default value
@ -313,8 +315,8 @@ protected:
* @param fDefault (true or false)
* @return command-line argument or default value
*/
bool GetBoolArg(const std::string& strArg, bool fDefault) const;
std::optional<bool> GetBoolArg(const std::string& strArg) const;
bool GetBoolArg(const std::string& strArg, bool fDefault) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
std::optional<bool> GetBoolArg(const std::string& strArg) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Set an argument if it doesn't already have a value
@ -323,7 +325,7 @@ protected:
* @param strValue Value (e.g. "1")
* @return true if argument gets set, false if it already had a value
*/
bool SoftSetArg(const std::string& strArg, const std::string& strValue);
bool SoftSetArg(const std::string& strArg, const std::string& strValue) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Set a boolean argument if it doesn't already have a value
@ -332,97 +334,97 @@ protected:
* @param fValue Value (e.g. false)
* @return true if argument gets set, false if it already had a value
*/
bool SoftSetBoolArg(const std::string& strArg, bool fValue);
bool SoftSetBoolArg(const std::string& strArg, bool fValue) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
// Forces an arg setting. Called by SoftSetArg() if the arg hasn't already
// been set. Also called directly in testing.
void ForceSetArg(const std::string& strArg, const std::string& strValue);
void ForceSetArg(const std::string& strArg, const std::string& strValue) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Returns the appropriate chain type from the program arguments.
* @return ChainType::MAIN by default; raises runtime error if an invalid
* combination, or unknown chain is given.
*/
ChainType GetChainType() const;
ChainType GetChainType() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Returns the appropriate chain type string from the program arguments.
* @return ChainType::MAIN string by default; raises runtime error if an
* invalid combination is given.
*/
std::string GetChainTypeString() const;
std::string GetChainTypeString() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Add argument
*/
void AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat);
void AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Add subcommand
*/
void AddCommand(const std::string& cmd, const std::string& help);
void AddCommand(const std::string& cmd, const std::string& help) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Add many hidden arguments
*/
void AddHiddenArgs(const std::vector<std::string>& args);
void AddHiddenArgs(const std::vector<std::string>& args) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Clear available arguments
*/
void ClearArgs();
void ClearArgs() EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Check CLI command args
*
* @throws std::runtime_error when multiple CLI_COMMAND arguments are specified
*/
void CheckMultipleCLIArgs() const;
void CheckMultipleCLIArgs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get the help string
*/
std::string GetHelpMessage() const;
std::string GetHelpMessage() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Return Flags for known arg.
* Return default flags for unknown arg.
*/
std::optional<unsigned int> GetArgFlags(const std::string& name) const;
std::optional<unsigned int> GetArgFlags(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Set default flags to return for an unknown arg.
*/
void SetDefaultFlags(std::optional<unsigned int>);
void SetDefaultFlags(std::optional<unsigned int>) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get settings file path, or return false if read-write settings were
* disabled with -nosettings.
*/
bool GetSettingsPath(fs::path* filepath = nullptr, bool temp = false, bool backup = false) const;
bool GetSettingsPath(fs::path* filepath = nullptr, bool temp = false, bool backup = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Read settings file. Push errors to vector, or log them if null.
*/
bool ReadSettingsFile(std::vector<std::string>* errors = nullptr);
bool ReadSettingsFile(std::vector<std::string>* errors = nullptr) EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Write settings file or backup settings file. Push errors to vector, or
* log them if null.
*/
bool WriteSettingsFile(std::vector<std::string>* errors = nullptr, bool backup = false) const;
bool WriteSettingsFile(std::vector<std::string>* errors = nullptr, bool backup = false) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Get current setting from config file or read/write settings file,
* ignoring nonpersistent command line or forced settings values.
*/
common::SettingsValue GetPersistentSetting(const std::string& name) const;
common::SettingsValue GetPersistentSetting(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
/**
* Access settings with lock held.
*/
template <typename Fn>
void LockSettings(Fn&& fn)
void LockSettings(Fn&& fn) EXCLUSIVE_LOCKS_REQUIRED(!cs_args)
{
LOCK(cs_args);
fn(m_settings);
@ -432,16 +434,21 @@ protected:
* Log the config file options and the command line arguments,
* useful for troubleshooting.
*/
void LogArgs() const;
void LogArgs() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
private:
// Internal helpers, for use by callers that already hold `cs_args`.
common::SettingsValue GetSetting_(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
std::optional<unsigned int> GetArgFlags_(const std::string& name) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
fs::path GetPathArg_(std::string arg, const fs::path& default_value = {}) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
/**
* Get data directory path
*
* @param net_specific Append network identifier to the returned path
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
*/
fs::path GetDataDir(bool net_specific) const;
fs::path GetDataDir(bool net_specific) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
/**
* Return -regtest/-signet/-testnet/-testnet4/-chain= setting as a ChainType enum if a
@ -449,13 +456,13 @@ private:
* name was set. Raise an exception if an invalid combination of flags was
* provided.
*/
std::variant<ChainType, std::string> GetChainArg() const;
std::variant<ChainType, std::string> GetChainArg() const EXCLUSIVE_LOCKS_REQUIRED(!cs_args);
// Helper function for LogArgs().
void logArgsPrefix(
const std::string& prefix,
const std::string& section,
const std::map<std::string, std::vector<common::SettingsValue>>& args) const;
const std::map<std::string, std::vector<common::SettingsValue>>& args) const EXCLUSIVE_LOCKS_REQUIRED(cs_args);
};
extern ArgsManager gArgs;

View File

@ -19,6 +19,7 @@
#include <filesystem>
#include <fstream>
#include <iostream>
#include <sstream>
#include <list>
#include <map>
#include <memory>
@ -99,7 +100,7 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file
}
for (const std::pair<std::string, std::string>& option : options) {
KeyInfo key = InterpretKey(option.first);
std::optional<unsigned int> flags = GetArgFlags('-' + key.name);
std::optional<unsigned int> flags = GetArgFlags_('-' + key.name);
if (!IsConfSupported(key, error)) return false;
if (flags) {
std::optional<common::SettingsValue> value = InterpretValue(key, &option.second, *flags, error);
@ -119,13 +120,26 @@ 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)
{
{
LOCK(cs_args);
m_settings.ro_config.clear();
m_config_sections.clear();
m_config_path = AbsPathForConfigVal(*this, GetPathArg("-conf", BITCOIN_CONF_FILENAME), /*net_specific=*/false);
const auto conf_val = GetPathArg_("-conf", BITCOIN_CONF_FILENAME);
m_config_path = (conf_val.is_absolute() || conf_val.empty()) ? conf_val : fsbridge::AbsPathJoin(GetDataDir(/*net_specific=*/false), conf_val);
}
const auto conf_path{GetConfigFilePath()};

View File

@ -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,12 +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_network;
using ArgsManager::m_settings;
};
//! Test GetSetting and GetArg type coercion, negation, and default value handling.
@ -217,29 +201,34 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters)
const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"};
std::string error;
LOCK(testArgs.cs_args);
testArgs.SetupArgs({a, b, ccc, d});
BOOST_CHECK(testArgs.ParseParameters(0, argv_test, error));
BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && testArgs.m_settings.ro_config.empty());
testArgs.LockSettings([&](const common::Settings& s) {
BOOST_CHECK(s.command_line_options.empty() && s.ro_config.empty());
});
BOOST_CHECK(testArgs.ParseParameters(1, argv_test, error));
BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && testArgs.m_settings.ro_config.empty());
testArgs.LockSettings([&](const common::Settings& s) {
BOOST_CHECK(s.command_line_options.empty() && s.ro_config.empty());
});
BOOST_CHECK(testArgs.ParseParameters(7, argv_test, error));
// 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.m_settings.command_line_options.size() == 3 && testArgs.m_settings.ro_config.empty());
BOOST_CHECK(testArgs.IsArgSet("-a") && testArgs.IsArgSet("-b") && testArgs.IsArgSet("-ccc")
&& !testArgs.IsArgSet("f") && !testArgs.IsArgSet("-d"));
BOOST_CHECK(testArgs.m_settings.command_line_options.contains("a") && testArgs.m_settings.command_line_options.contains("b") && testArgs.m_settings.command_line_options.contains("ccc")
&& !testArgs.m_settings.command_line_options.contains("f") && !testArgs.m_settings.command_line_options.contains("d"));
testArgs.LockSettings([&](const common::Settings& s) {
BOOST_CHECK(s.command_line_options.size() == 3 && s.ro_config.empty());
BOOST_CHECK(s.command_line_options.contains("a") && s.command_line_options.contains("b") && s.command_line_options.contains("ccc")
&& !s.command_line_options.contains("f") && !s.command_line_options.contains("d"));
BOOST_CHECK(testArgs.m_settings.command_line_options["a"].size() == 1);
BOOST_CHECK(testArgs.m_settings.command_line_options["a"].front().get_str() == "");
BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].size() == 2);
BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].front().get_str() == "argument");
BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].back().get_str() == "multiple");
BOOST_CHECK(s.command_line_options.at("a").size() == 1);
BOOST_CHECK(s.command_line_options.at("a").front().get_str() == "");
BOOST_CHECK(s.command_line_options.at("ccc").size() == 2);
BOOST_CHECK(s.command_line_options.at("ccc").front().get_str() == "argument");
BOOST_CHECK(s.command_line_options.at("ccc").back().get_str() == "multiple");
});
BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2);
}
@ -331,7 +320,6 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
const char *argv_test[] = {
"ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"};
std::string error;
LOCK(testArgs.cs_args);
testArgs.SetupArgs({a, b, c, d, e, f});
BOOST_CHECK(testArgs.ParseParameters(7, argv_test, error));
@ -340,8 +328,9 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg)
BOOST_CHECK(testArgs.IsArgSet({'-', opt}) || !opt);
// Nothing else should be in the map
BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 6 &&
testArgs.m_settings.ro_config.empty());
testArgs.LockSettings([&](const common::Settings& s) {
BOOST_CHECK(s.command_line_options.size() == 6 && s.ro_config.empty());
});
// The -no prefix should get stripped on the way in.
BOOST_CHECK(!testArgs.IsArgSet("-nob"));
@ -437,7 +426,6 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
"iii=2\n";
TestArgsManager test_args;
LOCK(test_args.cs_args);
const auto a = std::make_pair("-a", ArgsManager::ALLOW_ANY);
const auto b = std::make_pair("-b", ArgsManager::ALLOW_ANY);
const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_ANY);
@ -454,24 +442,26 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
// expectation: a, b, ccc, d, fff, ggg, h, i end up in map
// so do sec1.ccc, sec1.d, sec1.h, sec2.ccc, sec2.iii
BOOST_CHECK(test_args.m_settings.command_line_options.empty());
BOOST_CHECK(test_args.m_settings.ro_config.size() == 3);
BOOST_CHECK(test_args.m_settings.ro_config[""].size() == 8);
BOOST_CHECK(test_args.m_settings.ro_config["sec1"].size() == 3);
BOOST_CHECK(test_args.m_settings.ro_config["sec2"].size() == 2);
test_args.LockSettings([&](const common::Settings& s) {
BOOST_CHECK(s.command_line_options.empty());
BOOST_CHECK(s.ro_config.size() == 3);
BOOST_CHECK(s.ro_config.at("").size() == 8);
BOOST_CHECK(s.ro_config.at("sec1").size() == 3);
BOOST_CHECK(s.ro_config.at("sec2").size() == 2);
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("a"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("b"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("ccc"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("d"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("fff"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("ggg"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("h"));
BOOST_CHECK(test_args.m_settings.ro_config[""].contains("i"));
BOOST_CHECK(test_args.m_settings.ro_config["sec1"].contains("ccc"));
BOOST_CHECK(test_args.m_settings.ro_config["sec1"].contains("h"));
BOOST_CHECK(test_args.m_settings.ro_config["sec2"].contains("ccc"));
BOOST_CHECK(test_args.m_settings.ro_config["sec2"].contains("iii"));
BOOST_CHECK(s.ro_config.at("").contains("a"));
BOOST_CHECK(s.ro_config.at("").contains("b"));
BOOST_CHECK(s.ro_config.at("").contains("ccc"));
BOOST_CHECK(s.ro_config.at("").contains("d"));
BOOST_CHECK(s.ro_config.at("").contains("fff"));
BOOST_CHECK(s.ro_config.at("").contains("ggg"));
BOOST_CHECK(s.ro_config.at("").contains("h"));
BOOST_CHECK(s.ro_config.at("").contains("i"));
BOOST_CHECK(s.ro_config.at("sec1").contains("ccc"));
BOOST_CHECK(s.ro_config.at("sec1").contains("h"));
BOOST_CHECK(s.ro_config.at("sec2").contains("ccc"));
BOOST_CHECK(s.ro_config.at("sec2").contains("iii"));
});
BOOST_CHECK(test_args.IsArgSet("-a"));
BOOST_CHECK(test_args.IsArgSet("-b"));
@ -579,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");
@ -604,25 +597,26 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream)
BOOST_AUTO_TEST_CASE(util_GetArg)
{
TestArgsManager testArgs;
LOCK(testArgs.cs_args);
testArgs.m_settings.command_line_options.clear();
testArgs.m_settings.command_line_options["strtest1"] = {"string..."};
// strtest2 undefined on purpose
testArgs.m_settings.command_line_options["inttest1"] = {"12345"};
testArgs.m_settings.command_line_options["inttest2"] = {"81985529216486895"};
// inttest3 undefined on purpose
testArgs.m_settings.command_line_options["booltest1"] = {""};
// booltest2 undefined on purpose
testArgs.m_settings.command_line_options["booltest3"] = {"0"};
testArgs.m_settings.command_line_options["booltest4"] = {"1"};
testArgs.LockSettings([&](common::Settings& s) {
s.command_line_options.clear();
s.command_line_options["strtest1"] = {"string..."};
// strtest2 undefined on purpose
s.command_line_options["inttest1"] = {"12345"};
s.command_line_options["inttest2"] = {"81985529216486895"};
// inttest3 undefined on purpose
s.command_line_options["booltest1"] = {""};
// booltest2 undefined on purpose
s.command_line_options["booltest3"] = {"0"};
s.command_line_options["booltest4"] = {"1"};
// priorities
testArgs.m_settings.command_line_options["pritest1"] = {"a", "b"};
testArgs.m_settings.ro_config[""]["pritest2"] = {"a", "b"};
testArgs.m_settings.command_line_options["pritest3"] = {"a"};
testArgs.m_settings.ro_config[""]["pritest3"] = {"b"};
testArgs.m_settings.command_line_options["pritest4"] = {"a","b"};
testArgs.m_settings.ro_config[""]["pritest4"] = {"c","d"};
// priorities
s.command_line_options["pritest1"] = {"a", "b"};
s.ro_config[""]["pritest2"] = {"a", "b"};
s.command_line_options["pritest3"] = {"a"};
s.ro_config[""]["pritest3"] = {"b"};
s.command_line_options["pritest4"] = {"a","b"};
s.ro_config[""]["pritest4"] = {"c","d"};
});
BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string...");
BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default");
@ -815,16 +809,16 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup)
ForEachMergeSetup([&](const ActionList& arg_actions, const ActionList& conf_actions, bool soft_set, bool force_set,
const std::string& section, const std::string& network, bool net_specific) {
TestArgsManager parser;
LOCK(parser.cs_args);
std::string desc = "net=";
desc += network;
parser.m_network = network;
parser.SelectConfigNetwork(network);
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"};
@ -949,7 +943,6 @@ BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup)
ForEachMergeSetup([&](const ActionList& arg_actions, const ActionList& conf_actions) {
TestArgsManager parser;
LOCK(parser.cs_args);
parser.AddArg("-regtest", "regtest", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
parser.AddArg("-testnet4", "testnet4", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@ -1027,14 +1020,14 @@ BOOST_AUTO_TEST_CASE(util_ReadWriteSettings)
// Test writing setting.
TestArgsManager args1;
args1.ForceSetArg("-datadir", fs::PathToString(m_path_root));
args1.LockSettings([&](common::Settings& settings) { settings.rw_settings["name"] = "value"; });
args1.LockSettings([&](common::Settings& s) { s.rw_settings["name"] = "value"; });
args1.WriteSettingsFile();
// Test reading setting.
TestArgsManager args2;
args2.ForceSetArg("-datadir", fs::PathToString(m_path_root));
args2.ReadSettingsFile();
args2.LockSettings([&](common::Settings& settings) { BOOST_CHECK_EQUAL(settings.rw_settings["name"].get_str(), "value"); });
args2.LockSettings([&](common::Settings& s) { BOOST_CHECK_EQUAL(s.rw_settings["name"].get_str(), "value"); });
// Test error logging, and remove previously written setting.
{