Skip to content

Commit 4ce60d8

Browse files
stickies-vstickies-v
committed
tinyformat: remove std::string overload
All (non-test) format strings are known at compile-time, so we don't need the std::string overload. Remove it so users use the safer util::ConstevalFormatString where possible, or the bilingual_str overload where applicable.
1 parent 104c805 commit 4ce60d8

File tree

11 files changed

+37
-40
lines changed

11 files changed

+37
-40
lines changed

‎doc/developer-notes.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,9 @@ Strings and formatting
950950
951951
- Do not use it when passing strings to `tfm::format`, `strprintf`, `LogInfo`, `LogDebug`, etc.
952952
953-
- *Rationale*: This is redundant. Tinyformat handles strings.
953+
- *Rationale*: Tinyformat handles string parameters. Format strings
954+
should be known at compile-time, so using a string literal or
955+
`constexpr const char*` allows for compile-time validation.
954956
955957
- Do not use it to convert to `QString`. Use `QString::fromStdString()`.
956958

‎src/clientversion.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ std::string FormatSubVersion(const std::string& name, int nClientVersion, const
7171

7272
std::string CopyrightHolders(const std::string& strPrefix)
7373
{
74-
const auto copyright_devs = strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION);
74+
const auto copyright_devs = strprintf(_(COPYRIGHT_HOLDERS), COPYRIGHT_HOLDERS_SUBSTITUTION).translated;
7575
std::string strCopyrightHolders = strPrefix + copyright_devs;
7676

7777
// Make sure Bitcoin Core copyright is not removed by accident
@@ -85,15 +85,17 @@ std::string LicenseInfo()
8585
{
8686
const std::string URL_SOURCE_CODE = "<https://github.com/bitcoin/bitcoin>";
8787

88-
return CopyrightHolders(strprintf(_("Copyright (C) %i-%i").translated, 2009, COPYRIGHT_YEAR) + " ") + "\n" +
88+
return CopyrightHolders(strprintf(_("Copyright (C) %i-%i"), 2009, COPYRIGHT_YEAR).translated + " ") + "\n" +
8989
"\n" +
9090
strprintf(_("Please contribute if you find %s useful. "
91-
"Visit %s for further information about the software.").translated, PACKAGE_NAME, "<" PACKAGE_URL ">") +
91+
"Visit %s for further information about the software."),
92+
PACKAGE_NAME, "<" PACKAGE_URL ">")
93+
.translated +
9294
"\n" +
93-
strprintf(_("The source code is available from %s.").translated, URL_SOURCE_CODE) +
95+
strprintf(_("The source code is available from %s."), URL_SOURCE_CODE).translated +
9496
"\n" +
9597
"\n" +
9698
_("This is experimental software.").translated + "\n" +
97-
strprintf(_("Distributed under the MIT software license, see the accompanying file %s or %s").translated, "COPYING", "<https://opensource.org/licenses/MIT>") +
99+
strprintf(_("Distributed under the MIT software license, see the accompanying file %s or %s"), "COPYING", "<https://opensource.org/licenses/MIT>").translated +
98100
"\n";
99101
}

‎src/init.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
617617
strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)",
618618
Ticks<std::chrono::seconds>(DEFAULT_MAX_TIP_AGE)),
619619
ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
620-
argsman.AddArg("-printpriority", strprintf("Log transaction fee rate in " + CURRENCY_UNIT + "/kvB when mining blocks (default: %u)", DEFAULT_PRINT_MODIFIED_FEE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
620+
argsman.AddArg("-printpriority", strprintf("Log transaction fee rate in %s/kvB when mining blocks (default: %u)", CURRENCY_UNIT, DEFAULT_PRINT_MODIFIED_FEE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
621621
argsman.AddArg("-uacomment=<cmt>", "Append comment to the user agent string", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
622622

623623
SetupChainParamsBaseOptions(argsman);

‎src/test/fuzz/strprintf.cpp‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,27 +52,27 @@ FUZZ_TARGET(str_printf)
5252
CallOneOf(
5353
fuzzed_data_provider,
5454
[&] {
55-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
55+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeRandomLengthString(32));
5656
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
5757
},
5858
[&] {
59-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
59+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
6060
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
6161
},
6262
[&] {
63-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
63+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<signed char>());
6464
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
6565
},
6666
[&] {
67-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
67+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<unsigned char>());
6868
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
6969
},
7070
[&] {
71-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
71+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<char>());
7272
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<char>());
7373
},
7474
[&] {
75-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeBool());
75+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeBool());
7676
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeBool());
7777
});
7878
} catch (const tinyformat::format_error&) {
@@ -98,35 +98,35 @@ FUZZ_TARGET(str_printf)
9898
CallOneOf(
9999
fuzzed_data_provider,
100100
[&] {
101-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
101+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeFloatingPoint<float>());
102102
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
103103
},
104104
[&] {
105-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
105+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeFloatingPoint<double>());
106106
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
107107
},
108108
[&] {
109-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
109+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<int16_t>());
110110
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
111111
},
112112
[&] {
113-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
113+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<uint16_t>());
114114
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
115115
},
116116
[&] {
117-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
117+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<int32_t>());
118118
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
119119
},
120120
[&] {
121-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
121+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<uint32_t>());
122122
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
123123
},
124124
[&] {
125-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
125+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<int64_t>());
126126
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
127127
},
128128
[&] {
129-
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
129+
(void)strprintf(format_string.c_str(), fuzzed_data_provider.ConsumeIntegral<uint64_t>());
130130
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
131131
});
132132
} catch (const tinyformat::format_error&) {

‎src/test/txrequest_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,10 @@ class Scenario
178178
size_t real_total = runner.txrequest.Count(peer);
179179
size_t real_candidates = runner.txrequest.CountCandidates(peer);
180180
size_t real_inflight = runner.txrequest.CountInFlight(peer);
181-
BOOST_CHECK_MESSAGE(real_total == total, strprintf("[" + comment + "] total %i (%i expected)", real_total, total));
182-
BOOST_CHECK_MESSAGE(real_inflight == inflight, strprintf("[" + comment + "] inflight %i (%i expected)", real_inflight, inflight));
183-
BOOST_CHECK_MESSAGE(real_candidates == candidates, strprintf("[" + comment + "] candidates %i (%i expected)", real_candidates, candidates));
184-
BOOST_CHECK_MESSAGE(ret == expected, "[" + comment + "] mismatching requestables");
181+
BOOST_CHECK_MESSAGE(real_total == total, strprintf("[%s] total %i (%i expected)", comment, real_total, total));
182+
BOOST_CHECK_MESSAGE(real_inflight == inflight, strprintf("[%s] inflight %i (%i expected)", comment, real_inflight, inflight));
183+
BOOST_CHECK_MESSAGE(real_candidates == candidates, strprintf("[%s] candidates %i (%i expected)", comment, real_candidates, candidates));
184+
BOOST_CHECK_MESSAGE(ret == expected, strprintf("[%s] mismatching requestables", comment));
185185
});
186186
}
187187

‎src/tinyformat.h‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,13 +1147,6 @@ TINYFORMAT_FOREACH_ARGNUM(TINYFORMAT_MAKE_FORMAT_FUNCS)
11471147
#endif
11481148

11491149
// Added for Bitcoin Core
1150-
template<typename... Args>
1151-
std::string format(const std::string &fmt, const Args&... args)
1152-
{
1153-
std::ostringstream oss;
1154-
format(oss, fmt.c_str(), args...);
1155-
return oss.str();
1156-
}
11571150
template <typename... Args>
11581151
std::string format(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
11591152
{

‎src/util/translation.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ bilingual_str format(const bilingual_str& fmt, const Args&... args)
5959
return arg;
6060
}
6161
}};
62-
return bilingual_str{tfm::format(fmt.original, translate_arg(args, false)...),
63-
tfm::format(fmt.translated, translate_arg(args, true)...)};
62+
return bilingual_str{tfm::format(fmt.original.c_str(), translate_arg(args, false)...),
63+
tfm::format(fmt.translated.c_str(), translate_arg(args, true)...)};
6464
}
6565
} // namespace tinyformat
6666

‎src/wallet/rpc/backup.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ RPCHelpMan importwallet()
534534

535535
// Use uiInterface.ShowProgress instead of pwallet.ShowProgress because pwallet.ShowProgress has a cancel button tied to AbortRescan which
536536
// we don't want for this progress bar showing the import progress. uiInterface.ShowProgress does not have a cancel button.
537-
pwallet->chain().showProgress(strprintf("%s " + _("Importing…").translated, pwallet->GetDisplayName()), 0, false); // show progress dialog in GUI
537+
pwallet->chain().showProgress(strprintf("%s %s", pwallet->GetDisplayName(), _("Importing…").translated), 0, false); // show progress dialog in GUI
538538
std::vector<std::tuple<CKey, int64_t, bool, std::string>> keys;
539539
std::vector<std::pair<CScript, int64_t>> scripts;
540540
while (file.good()) {

‎src/wallet/test/fuzz/notifications.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void ImportDescriptors(CWallet& wallet, const std::string& seed_insecure)
6464

6565
for (const std::string& desc_fmt : DESCS) {
6666
for (bool internal : {true, false}) {
67-
const auto descriptor{(strprintf)(desc_fmt, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})};
67+
const auto descriptor{(strprintf)(desc_fmt.c_str(), "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})};
6868

6969
FlatSigningProvider keys;
7070
std::string error;

‎src/wallet/wallet.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,7 +1891,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
18911891
fast_rescan_filter ? "fast variant using block filters" : "slow variant inspecting all blocks");
18921892

18931893
fAbortRescan = false;
1894-
ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption)
1894+
ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), 0); // show rescan progress in GUI as dialog or on splashscreen, if rescan required on startup (e.g. due to corruption)
18951895
uint256 tip_hash = WITH_LOCK(cs_wallet, return GetLastBlockHash());
18961896
uint256 end_hash = tip_hash;
18971897
if (max_height) chain().findAncestorByHeight(tip_hash, *max_height, FoundBlock().hash(end_hash));
@@ -1906,7 +1906,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
19061906
m_scanning_progress = 0;
19071907
}
19081908
if (block_height % 100 == 0 && progress_end - progress_begin > 0.0) {
1909-
ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), std::max(1, std::min(99, (int)(m_scanning_progress * 100))));
1909+
ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), std::max(1, std::min(99, (int)(m_scanning_progress * 100))));
19101910
}
19111911

19121912
bool next_interval = reserver.now() >= current_time + INTERVAL_TIME;
@@ -2003,7 +2003,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
20032003
WalletLogPrintf("Scanning current mempool transactions.\n");
20042004
WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this));
20052005
}
2006-
ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 100); // hide progress dialog in GUI
2006+
ShowProgress(strprintf("%s %s", GetDisplayName(), _("Rescanning…").translated), 100); // hide progress dialog in GUI
20072007
if (block_height && fAbortRescan) {
20082008
WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height, progress_current);
20092009
result.status = ScanResult::USER_ABORT;

0 commit comments

Comments
 (0)