From f766189e1171809c50d0e961938cc8a5857591e8 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 25 Jul 2022 15:01:16 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#25611: univalue: Avoid brittle, narrowing and verbose integral type confusions fa23c197509f692a815193acc1b50bad2fcbedfe univalue: Avoid narrowing and verbose int constructors (MacroFake) fa3a9a1e8d9b6dffda772e97c279f3c0af6813f9 rpc: Select int-UniValue constructor for enum value in upgradewallet RPC (MacroFake) Pull request description: As UniValue provides several constructors for integral types, the compiler is unable to select one if the passed type does not exactly match. This is unintuitive for developers and forces them to write verbose and brittle code. (Refer to `-Wnarrowing` compiler warning) For example, there are many places where an unsigned int is cast to a signed int. While the cast is safe in practice, it is still needlessly verbose and confusing as the value can never be negative. In fact it might even be unsafe if the unsigned value is large enough to map to a negative signed one. Fix this issue and other (minor) type issues. ACKs for top commit: aureleoules: ACK fa23c197509f692a815193acc1b50bad2fcbedfe. Tree-SHA512: 7d99b5b90c7d8eed2e3448167255a59e817dd6b8fcfc1b17c69ddefd0db33d1bf4344fbcd8b7f8685b58182c0f572ab9ffa99467afa666ac21843df7ea645033 --- src/rpc/net.cpp | 14 ++++++------ src/univalue/include/univalue.h | 40 ++++++++++++++++----------------- src/wallet/rpc/wallet.cpp | 2 +- 3 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f6a965ffaa5d..eec54fa38d21 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -64,7 +64,7 @@ static RPCHelpMan getconnectioncount() const NodeContext& node = EnsureAnyNodeContext(request.context); const CConnman& connman = EnsureConnman(node); - return (int)connman.GetNodeCount(ConnectionDirection::Both); + return connman.GetNodeCount(ConnectionDirection::Both); }, }; } @@ -684,12 +684,12 @@ static RPCHelpMan getnetworkinfo() obj.pushKV("timeoffset", GetTimeOffset()); if (node.connman) { obj.pushKV("networkactive", node.connman->GetNetworkActive()); - obj.pushKV("connections", (int)node.connman->GetNodeCount(ConnectionDirection::Both)); - obj.pushKV("connections_in", (int)node.connman->GetNodeCount(ConnectionDirection::In)); - obj.pushKV("connections_out", (int)node.connman->GetNodeCount(ConnectionDirection::Out)); - obj.pushKV("connections_mn", (int)node.connman->GetNodeCount(ConnectionDirection::Verified)); - obj.pushKV("connections_mn_in", (int)node.connman->GetNodeCount(ConnectionDirection::VerifiedIn)); - obj.pushKV("connections_mn_out", (int)node.connman->GetNodeCount(ConnectionDirection::VerifiedOut)); + obj.pushKV("connections", node.connman->GetNodeCount(ConnectionDirection::Both)); + obj.pushKV("connections_in", node.connman->GetNodeCount(ConnectionDirection::In)); + obj.pushKV("connections_out", node.connman->GetNodeCount(ConnectionDirection::Out)); + obj.pushKV("connections_mn", node.connman->GetNodeCount(ConnectionDirection::Verified)); + obj.pushKV("connections_mn_in", node.connman->GetNodeCount(ConnectionDirection::VerifiedIn)); + obj.pushKV("connections_mn_out", node.connman->GetNodeCount(ConnectionDirection::VerifiedOut)); std::string_view sem_str = SEMToString(node.connman->GetSocketEventsMode()); CHECK_NONFATAL(sem_str != "unknown"); obj.pushKV("socketevents", std::string(sem_str)); diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index fc5cf402be3c..a02903baadd8 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -23,27 +23,25 @@ class UniValue { typ = initialType; val = initialStr; } - UniValue(uint64_t val_) { - setInt(val_); - } - UniValue(int64_t val_) { - setInt(val_); - } - UniValue(bool val_) { - setBool(val_); - } - UniValue(int val_) { - setInt(val_); - } - UniValue(double val_) { - setFloat(val_); - } - UniValue(const std::string& val_) { - setStr(val_); - } - UniValue(const char *val_) { - std::string s(val_); - setStr(s); + template >, + std::enable_if_t || // setFloat + std::is_same_v || // setBool + std::is_signed_v || std::is_unsigned_v || // setInt + std::is_constructible_v, // setStr + bool> = true> + UniValue(Ref&& val) + { + if constexpr (std::is_floating_point_v) { + setFloat(val); + } else if constexpr (std::is_same_v) { + setBool(val); + } else if constexpr (std::is_signed_v) { + setInt(int64_t{val}); + } else if constexpr (std::is_unsigned_v) { + setInt(uint64_t{val}); + } else { + setStr(std::string{std::forward(val)}); + } } void clear(); diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 632c030ee8d9..49ad27e80a42 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -848,7 +848,7 @@ static RPCHelpMan upgradewallet() "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified.\n" "New keys may be generated and a new wallet backup will need to be made.", { - {"version", RPCArg::Type::NUM, RPCArg::Default{FEATURE_LATEST}, "The version number to upgrade to. Default is the latest wallet version."} + {"version", RPCArg::Type::NUM, RPCArg::Default{int{FEATURE_LATEST}}, "The version number to upgrade to. Default is the latest wallet version."} }, RPCResult{ RPCResult::Type::OBJ, "", "",