mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-07-23 22:58:59 +02:00
Merge bitcoin/bitcoin#35470: argsman: Prevent duplicate option registration across categories
32df86f1d8argsman: Prevent duplicate option registration across categories (Pablo Martin) Pull request description: Follow-up to #28802 This PR enforces the invariant that option names are unique across categories. -<ins>_**Rationale**_</ins>: - While adapting the `argsman_tests.cpp` cases introduced in #28802 for the GNU-style parsing changes proposed in #33540, I noticed that the same option name can currently be registered in multiple categories. - At present in `master`, this ambiguity is largely masked by the existing command-line parsing behaviour. However, it relies on assumptions about how options are interpreted based on their position. Future changes to option parsing, such as the GNU-style parsing proposed in #33540, may expose this ambiguity and lead to unexpected option resolution. - To avoid ambiguous option resolution and make the distinction between global and command-specific options explicit, this PR adds validation in `AddArg()` preventing the same option name from being registered across different categories. ACKs for top commit: sedited: ACK32df86f1d8ryanofsky: Code review ACK32df86f1d8. Thanks for the simplifications! Tree-SHA512: 5d3cb951bd90c46cbd8205a229d49336c3a29e5100b7c50c5cba66979dd3cbd480ed42593caa272c6205eb7ac503af631a7e3531af0b726334e3448e17d5c0d5
This commit is contained in:
@@ -668,6 +668,11 @@ void ArgsManager::AddArg(const std::string& name, const std::string& help, unsig
|
||||
std::string arg_name = name.substr(0, eq_index);
|
||||
|
||||
LOCK(cs_args);
|
||||
|
||||
for (const auto& arg_map : m_available_args) {
|
||||
Assert(!arg_map.second.contains(arg_name));
|
||||
}
|
||||
|
||||
std::map<std::string, Arg>& arg_map = m_available_args[cat];
|
||||
auto ret = arg_map.emplace(arg_name, Arg{name.substr(eq_index, name.size() - eq_index), help, flags});
|
||||
assert(ret.second); // Make sure an insertion actually happened
|
||||
|
||||
Reference in New Issue
Block a user