Merge bitcoin/bitcoin#32983: rpc: refactor: use string_view in Arg/MaybeArg

b63428ac9c rpc: refactor: use more (Maybe)Arg<std::string_view> (stickies-v)
037830ca0d refactor: increase string_view usage (stickies-v)
b3bf18f0ba rpc: refactor: use string_view in Arg/MaybeArg (stickies-v)

Pull request description:

  The `RPCHelpMan::{Arg,MaybeArg}` helpers avoid copying (potentially) large strings by returning them as `const std::string*` (`MaybeArg`) or `const std::string&` (`Arg`). For `MaybeArg`, this has the not-so-nice effect that users need to deal with raw pointers, potentially also requiring new functions (e.g. [`EnsureUniqueWalletName` ](d127b25199 (diff-d8bfcfbdd5fa7d5c52d38c1fe5eeac9ce5c5a794cdfaf683585140fa70a32374R32))) with raw pointers being implemented.

  This PR aims to improve on this by returning a trivially copyable `std::string_view` (`Arg`) or `std::optional<std::string_view>` (`MaybeArg`), modernizing the interface without introducing any additional copying overhead. In doing so, it also generalizes whether we return by value or by pointer/reference using `std::is_trivially_copyable_v` instead of defining the types manually.

  In cases where functions currently take a `const std::string&` and it would be too much work / touching consensus logic to update them (`signmessage.cpp`), a `std::string` copy is made (which was already happening anyway).

  The last 2 commits increase usage of the `{Arg,MaybeArg}<std::string_view>` helpers, and could be dropped/pruned if anything turns out to be controversial - I just think it's a nice little cleanup.

ACKs for top commit:
  maflcko:
    re-ACK b63428ac9c 🎉
  achow101:
    ACK b63428ac9c
  pablomartin4btc:
    re-ACK [b63428a](b63428ac9c)
  w0xlt:
    reACK b63428ac9c

Tree-SHA512: b4942c353a1658c22a88d8c9b402c288ad35265a3b88aa2072b1f9b6d921cd073194ed4b00b807cb48ca440f47c87ef3d8e0dd1a5d814be58fc7743f26288277
This commit is contained in:
Ava Chow
2025-10-24 10:33:51 -07:00
32 changed files with 148 additions and 150 deletions

View File

@@ -39,11 +39,12 @@
#include <algorithm>
#include <array>
#include <cstring>
#include <cmath>
#include <cstdint>
#include <cstring>
#include <functional>
#include <optional>
#include <string_view>
#include <unordered_map>
TRACEPOINT_SEMAPHORE(net, closed_connection);
@@ -331,7 +332,7 @@ bool IsLocal(const CService& addr)
return mapLocalHost.count(addr) > 0;
}
bool CConnman::AlreadyConnectedToHost(const std::string& host) const
bool CConnman::AlreadyConnectedToHost(std::string_view host) const
{
LOCK(m_nodes_mutex);
return std::ranges::any_of(m_nodes, [&host](CNode* node) { return node->m_addr_name == host; });
@@ -3589,11 +3590,11 @@ bool CConnman::AddNode(const AddedNodeParams& add)
return true;
}
bool CConnman::RemoveAddedNode(const std::string& strNode)
bool CConnman::RemoveAddedNode(std::string_view node)
{
LOCK(m_added_nodes_mutex);
for (auto it = m_added_node_params.begin(); it != m_added_node_params.end(); ++it) {
if (strNode == it->m_added_node) {
if (node == it->m_added_node) {
m_added_node_params.erase(it);
return true;
}
@@ -3652,7 +3653,7 @@ void CConnman::GetNodeStats(std::vector<CNodeStats>& vstats) const
}
}
bool CConnman::DisconnectNode(const std::string& strNode)
bool CConnman::DisconnectNode(std::string_view strNode)
{
LOCK(m_nodes_mutex);
auto it = std::ranges::find_if(m_nodes, [&strNode](CNode* node) { return node->m_addr_name == strNode; });