Skip to content

Commit f8364df

Browse files
author
MarcoFalke
committed
Merge #19176: refactor: Error message bilingual_str consistency
6fe9890 refactor: Change Node::initError to take bilingual_str (Wladimir J. van der Laan) 425e7cb refactor: Put`TryParsePermissionFlags` in anonymous namespace (Wladimir J. van der Laan) 77b79fa refactor: Error message bilingual_str consistency (Wladimir J. van der Laan) Pull request description: A straightforward and hopefully uncontroversial refactor to improve consistency. - Move the decision whether to translate an individual error message to where it is defined. This simplifies call sites: no more `InitError(Untranslated(SomeFunction(...)))`. - Make all functions in `util/error.h` consistently return a `bilingual_str`. We've decided to use this as error message type so let's roll with it. This has no functional changes: no messages are changed, no new translation messages are defined. Also make a function static that can be static. ACKs for top commit: MarcoFalke: ACK 6fe9890 🔣 hebasto: ACK 6fe9890, tested on Linux Mint 19.3 (x86_64). Tree-SHA512: 1dd123ef285c4b50bbc429b2f11c9a63aaa669a84955a0a9b8134e9dc141bc38f863f798e8982ac68bbe83170e1067a87d1a87fe7f791928b7914e10bbc2ef8d
2 parents 221873e + 6fe9890 commit f8364df

File tree

13 files changed

+57
-47
lines changed

13 files changed

+57
-47
lines changed

src/init.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,7 +1465,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
14651465
if (Lookup(strAddr, addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid())
14661466
AddLocal(addrLocal, LOCAL_MANUAL);
14671467
else
1468-
return InitError(Untranslated(ResolveErrMsg("externalip", strAddr)));
1468+
return InitError(ResolveErrMsg("externalip", strAddr));
14691469
}
14701470

14711471
// Read asmap file if configured
@@ -1904,21 +1904,21 @@ bool AppInitMain(const util::Ref& context, NodeContext& node)
19041904
for (const std::string& strBind : gArgs.GetArgs("-bind")) {
19051905
CService addrBind;
19061906
if (!Lookup(strBind, addrBind, GetListenPort(), false)) {
1907-
return InitError(Untranslated(ResolveErrMsg("bind", strBind)));
1907+
return InitError(ResolveErrMsg("bind", strBind));
19081908
}
19091909
connOptions.vBinds.push_back(addrBind);
19101910
}
19111911
for (const std::string& strBind : gArgs.GetArgs("-whitebind")) {
19121912
NetWhitebindPermissions whitebind;
1913-
std::string error;
1914-
if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(Untranslated(error));
1913+
bilingual_str error;
1914+
if (!NetWhitebindPermissions::TryParse(strBind, whitebind, error)) return InitError(error);
19151915
connOptions.vWhiteBinds.push_back(whitebind);
19161916
}
19171917

19181918
for (const auto& net : gArgs.GetArgs("-whitelist")) {
19191919
NetWhitelistPermissions subnet;
1920-
std::string error;
1921-
if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(Untranslated(error));
1920+
bilingual_str error;
1921+
if (!NetWhitelistPermissions::TryParse(net, subnet, error)) return InitError(error);
19221922
connOptions.vWhitelistedRange.push_back(subnet);
19231923
}
19241924

src/interfaces/node.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ namespace {
5656
class NodeImpl : public Node
5757
{
5858
public:
59-
void initError(const std::string& message) override { InitError(Untranslated(message)); }
59+
void initError(const bilingual_str& message) override { InitError(message); }
6060
bool parseParameters(int argc, const char* const argv[], std::string& error) override
6161
{
6262
return gArgs.ParseParameters(argc, argv, error);

src/interfaces/node.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class Node
4545
virtual ~Node() {}
4646

4747
//! Send init error.
48-
virtual void initError(const std::string& message) = 0;
48+
virtual void initError(const bilingual_str& message) = 0;
4949

5050
//! Set command line arguments.
5151
virtual bool parseParameters(int argc, const char* const argv[], std::string& error) = 0;

src/net_permissions.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{
1616
"mempool (allow requesting BIP35 mempool contents)",
1717
};
1818

19+
namespace {
20+
1921
// The parse the following format "perm1,perm2@xxxxxx"
20-
bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, std::string& error)
22+
bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, bilingual_str& error)
2123
{
2224
NetPermissionFlags flags = PF_NONE;
2325
const auto atSeparator = str.find('@');
@@ -48,18 +50,20 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output,
4850
else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY);
4951
else if (permission.length() == 0); // Allow empty entries
5052
else {
51-
error = strprintf(_("Invalid P2P permission: '%s'").translated, permission);
53+
error = strprintf(_("Invalid P2P permission: '%s'"), permission);
5254
return false;
5355
}
5456
}
5557
readen++;
5658
}
5759

5860
output = flags;
59-
error = "";
61+
error = Untranslated("");
6062
return true;
6163
}
6264

65+
}
66+
6367
std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags)
6468
{
6569
std::vector<std::string> strings;
@@ -71,7 +75,7 @@ std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags)
7175
return strings;
7276
}
7377

74-
bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error)
78+
bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error)
7579
{
7680
NetPermissionFlags flags;
7781
size_t offset;
@@ -84,17 +88,17 @@ bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermis
8488
return false;
8589
}
8690
if (addrBind.GetPort() == 0) {
87-
error = strprintf(_("Need to specify a port with -whitebind: '%s'").translated, strBind);
91+
error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind);
8892
return false;
8993
}
9094

9195
output.m_flags = flags;
9296
output.m_service = addrBind;
93-
error = "";
97+
error = Untranslated("");
9498
return true;
9599
}
96100

97-
bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error)
101+
bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error)
98102
{
99103
NetPermissionFlags flags;
100104
size_t offset;
@@ -104,12 +108,12 @@ bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermis
104108
CSubNet subnet;
105109
LookupSubNet(net, subnet);
106110
if (!subnet.IsValid()) {
107-
error = strprintf(_("Invalid netmask specified in -whitelist: '%s'").translated, net);
111+
error = strprintf(_("Invalid netmask specified in -whitelist: '%s'"), net);
108112
return false;
109113
}
110114

111115
output.m_flags = flags;
112116
output.m_subnet = subnet;
113-
error = "";
117+
error = Untranslated("");
114118
return true;
115119
}

src/net_permissions.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
#ifndef BITCOIN_NET_PERMISSIONS_H
1111
#define BITCOIN_NET_PERMISSIONS_H
1212

13+
struct bilingual_str;
14+
1315
extern const std::vector<std::string> NET_PERMISSIONS_DOC;
1416

1517
enum NetPermissionFlags
@@ -54,14 +56,14 @@ class NetPermissions
5456
class NetWhitebindPermissions : public NetPermissions
5557
{
5658
public:
57-
static bool TryParse(const std::string str, NetWhitebindPermissions& output, std::string& error);
59+
static bool TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error);
5860
CService m_service;
5961
};
6062

6163
class NetWhitelistPermissions : public NetPermissions
6264
{
6365
public:
64-
static bool TryParse(const std::string str, NetWhitelistPermissions& output, std::string& error);
66+
static bool TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error);
6567
CSubNet m_subnet;
6668
};
6769

src/qt/bitcoin.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ int GuiMain(int argc, char* argv[])
457457
SetupUIArgs();
458458
std::string error;
459459
if (!node->parseParameters(argc, argv, error)) {
460-
node->initError(strprintf("Error parsing command line arguments: %s\n", error));
460+
node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
461461
// Create a message box, because the gui has neither been created nor has subscribed to core signals
462462
QMessageBox::critical(nullptr, PACKAGE_NAME,
463463
// message can not be translated because translations have not been initialized
@@ -498,13 +498,13 @@ int GuiMain(int argc, char* argv[])
498498
/// 6. Determine availability of data directory and parse bitcoin.conf
499499
/// - Do not call GetDataDir(true) before this step finishes
500500
if (!CheckDataDirOption()) {
501-
node->initError(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")));
501+
node->initError(strprintf(Untranslated("Specified data directory \"%s\" does not exist.\n"), gArgs.GetArg("-datadir", "")));
502502
QMessageBox::critical(nullptr, PACKAGE_NAME,
503503
QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", ""))));
504504
return EXIT_FAILURE;
505505
}
506506
if (!node->readConfigFiles(error)) {
507-
node->initError(strprintf("Error reading configuration file: %s\n", error));
507+
node->initError(strprintf(Untranslated("Error reading configuration file: %s\n"), error));
508508
QMessageBox::critical(nullptr, PACKAGE_NAME,
509509
QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error)));
510510
return EXIT_FAILURE;
@@ -520,7 +520,7 @@ int GuiMain(int argc, char* argv[])
520520
try {
521521
node->selectParams(gArgs.GetChainName());
522522
} catch(std::exception &e) {
523-
node->initError(strprintf("%s\n", e.what()));
523+
node->initError(Untranslated(strprintf("%s\n", e.what())));
524524
QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(e.what()));
525525
return EXIT_FAILURE;
526526
}

src/rpc/util.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <tinyformat.h>
1111
#include <util/strencodings.h>
1212
#include <util/string.h>
13+
#include <util/translation.h>
1314

1415
#include <tuple>
1516

@@ -285,7 +286,7 @@ UniValue JSONRPCTransactionError(TransactionError terr, const std::string& err_s
285286
if (err_string.length() > 0) {
286287
return JSONRPCError(RPCErrorFromTransactionError(terr), err_string);
287288
} else {
288-
return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr));
289+
return JSONRPCError(RPCErrorFromTransactionError(terr), TransactionErrorString(terr).original);
289290
}
290291
}
291292

src/test/fuzz/kitchen_sink.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <test/fuzz/fuzz.h>
88
#include <test/fuzz/util.h>
99
#include <util/error.h>
10+
#include <util/translation.h>
1011

1112
#include <cstdint>
1213
#include <vector>

src/test/fuzz/net_permissions.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <test/fuzz/FuzzedDataProvider.h>
77
#include <test/fuzz/fuzz.h>
88
#include <test/fuzz/util.h>
9+
#include <util/translation.h>
910

1011
#include <cassert>
1112
#include <cstdint>
@@ -29,7 +30,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
2930
static_cast<NetPermissionFlags>(fuzzed_data_provider.ConsumeIntegral<uint32_t>());
3031

3132
NetWhitebindPermissions net_whitebind_permissions;
32-
std::string error_net_whitebind_permissions;
33+
bilingual_str error_net_whitebind_permissions;
3334
if (NetWhitebindPermissions::TryParse(s, net_whitebind_permissions, error_net_whitebind_permissions)) {
3435
(void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags);
3536
(void)NetPermissions::AddFlag(net_whitebind_permissions.m_flags, net_permission_flags);
@@ -39,7 +40,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
3940
}
4041

4142
NetWhitelistPermissions net_whitelist_permissions;
42-
std::string error_net_whitelist_permissions;
43+
bilingual_str error_net_whitelist_permissions;
4344
if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, error_net_whitelist_permissions)) {
4445
(void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags);
4546
(void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags);

src/test/netbase_tests.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <netbase.h>
77
#include <test/util/setup_common.h>
88
#include <util/strencodings.h>
9+
#include <util/translation.h>
910

1011
#include <string>
1112

@@ -325,15 +326,15 @@ BOOST_AUTO_TEST_CASE(netbase_parsenetwork)
325326

326327
BOOST_AUTO_TEST_CASE(netpermissions_test)
327328
{
328-
std::string error;
329+
bilingual_str error;
329330
NetWhitebindPermissions whitebindPermissions;
330331
NetWhitelistPermissions whitelistPermissions;
331332

332333
// Detect invalid white bind
333334
BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error));
334-
BOOST_CHECK(error.find("Cannot resolve -whitebind address") != std::string::npos);
335+
BOOST_CHECK(error.original.find("Cannot resolve -whitebind address") != std::string::npos);
335336
BOOST_CHECK(!NetWhitebindPermissions::TryParse("127.0.0.1", whitebindPermissions, error));
336-
BOOST_CHECK(error.find("Need to specify a port with -whitebind") != std::string::npos);
337+
BOOST_CHECK(error.original.find("Need to specify a port with -whitebind") != std::string::npos);
337338
BOOST_CHECK(!NetWhitebindPermissions::TryParse("", whitebindPermissions, error));
338339

339340
// If no permission flags, assume backward compatibility
@@ -377,11 +378,11 @@ BOOST_AUTO_TEST_CASE(netpermissions_test)
377378

378379
// Detect invalid flag
379380
BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,[email protected]:32", whitebindPermissions, error));
380-
BOOST_CHECK(error.find("Invalid P2P permission") != std::string::npos);
381+
BOOST_CHECK(error.original.find("Invalid P2P permission") != std::string::npos);
381382

382383
// Check whitelist error
383384
BOOST_CHECK(!NetWhitelistPermissions::TryParse("bloom,forcerelay,[email protected]:32", whitelistPermissions, error));
384-
BOOST_CHECK(error.find("Invalid netmask specified in -whitelist") != std::string::npos);
385+
BOOST_CHECK(error.original.find("Invalid netmask specified in -whitelist") != std::string::npos);
385386

386387
// Happy path for whitelist parsing
387388
BOOST_CHECK(NetWhitelistPermissions::TryParse("[email protected]", whitelistPermissions, error));

0 commit comments

Comments
 (0)