Skip to content

refactor: Check translatable format strings at compile-time #31061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/banman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ void BanMan::LoadBanlist()
{
LOCK(m_banned_mutex);

if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…").translated);
if (m_client_interface) m_client_interface->InitMessage(_("Loading banlist…"));

const auto start{SteadyClock::now()};
if (m_ban_db.Read(m_banned)) {
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoin-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ using util::ToString;
// just use a plain system_clock.
using CliClock = std::chrono::system_clock;

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
const TranslateFn G_TRANSLATION_FUN{nullptr};

static const char DEFAULT_RPCCONNECT[] = "127.0.0.1";
static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900;
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoin-tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static bool fCreateBlank;
static std::map<std::string,UniValue> registers;
static const int CONTINUE_EXECUTION=-1;

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
const TranslateFn G_TRANSLATION_FUN{nullptr};

static void SetupBitcoinTxArgs(ArgsManager &argsman)
{
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoin-util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

static const int CONTINUE_EXECUTION=-1;

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
const TranslateFn G_TRANSLATION_FUN{nullptr};

static void SetupBitcoinUtilArgs(ArgsManager &argsman)
{
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoin-wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

using util::Join;

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
const TranslateFn G_TRANSLATION_FUN{nullptr};

static void SetupWalletToolArgs(ArgsManager& argsman)
{
Expand Down
2 changes: 1 addition & 1 deletion src/bitcoind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@

using node::NodeContext;

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
const TranslateFn G_TRANSLATION_FUN{nullptr};

#if HAVE_DECL_FORK

Expand Down
2 changes: 1 addition & 1 deletion src/clientversion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ std::string LicenseInfo()
strprintf(_("The source code is available from %s."), URL_SOURCE_CODE).translated +
"\n" +
"\n" +
_("This is experimental software.").translated + "\n" +
_("This is experimental software.") + "\n" +
strprintf(_("Distributed under the MIT software license, see the accompanying file %s or %s"), "COPYING", "<https://opensource.org/licenses/MIT>").translated +
"\n";
}
12 changes: 6 additions & 6 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1244,8 +1244,8 @@ static ChainstateLoadResult InitAndLoadChainstate(
_("Error reading from database, shutting down."),
"", CClientUIInterface::MSG_ERROR);
};
uiInterface.InitMessage(_("Loading block index…").translated);
auto catch_exceptions = [](auto&& f) {
uiInterface.InitMessage(_("Loading block index…"));
auto catch_exceptions = [](auto&& f) -> ChainstateLoadResult {
try {
return f();
} catch (const std::exception& e) {
Expand All @@ -1255,7 +1255,7 @@ static ChainstateLoadResult InitAndLoadChainstate(
};
auto [status, error] = catch_exceptions([&] { return LoadChainstate(chainman, cache_sizes, options); });
if (status == node::ChainstateLoadStatus::SUCCESS) {
uiInterface.InitMessage(_("Verifying blocks…").translated);
uiInterface.InitMessage(_("Verifying blocks…"));
if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) {
LogWarning("pruned datadir may not have more than %d blocks; only checking available blocks\n",
MIN_BLOCKS_TO_KEEP);
Expand Down Expand Up @@ -1418,7 +1418,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)

// Initialize addrman
assert(!node.addrman);
uiInterface.InitMessage(_("Loading P2P addresses…").translated);
uiInterface.InitMessage(_("Loading P2P addresses…"));
auto addrman{LoadAddrman(*node.netgroupman, args)};
if (!addrman) return InitError(util::ErrorString(addrman));
node.addrman = std::move(*addrman);
Expand Down Expand Up @@ -1703,7 +1703,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (chainman.m_blockman.m_blockfiles_indexed) {
LOCK(cs_main);
for (Chainstate* chainstate : chainman.GetAll()) {
uiInterface.InitMessage(_("Pruning blockstore…").translated);
uiInterface.InitMessage(_("Pruning blockstore…"));
chainstate->PruneAndFlush();
}
}
Expand Down Expand Up @@ -1999,7 +1999,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
// ChainstateManager's active tip.
SetRPCWarmupFinished();

uiInterface.InitMessage(_("Done loading").translated);
uiInterface.InitMessage(_("Done loading"));

for (const auto& client : node.chain_clients) {
client->start(scheduler);
Expand Down
3 changes: 2 additions & 1 deletion src/kernel/bitcoinkernel.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Copyright (c) 2022 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <util/translation.h>

#include <functional>
#include <string>

// Define G_TRANSLATION_FUN symbol in libbitcoinkernel library so users of the
// library aren't required to export this symbol
extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
extern const TranslateFn G_TRANSLATION_FUN{nullptr};
2 changes: 1 addition & 1 deletion src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3296,7 +3296,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)
}

if (m_client_interface) {
m_client_interface->InitMessage(_("Starting network threads…").translated);
m_client_interface->InitMessage(_("Starting network threads…"));
}

fAddressesInitialized = true;
Expand Down
2 changes: 1 addition & 1 deletion src/qt/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include <string>

/** Translate string to current locale using Qt. */
extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = [](const char* psz) {
extern const TranslateFn G_TRANSLATION_FUN = [](const char* psz) {
return QCoreApplication::translate("bitcoin-core", psz).toStdString();
};

Expand Down
49 changes: 20 additions & 29 deletions src/test/fuzz/strprintf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@
#include <string>
#include <vector>

template <typename... Args>
void fuzz_fmt(const std::string& fmt, const Args&... args)
{
(void)tfm::format(tfm::RuntimeFormat{fmt}, args...);
}

FUZZ_TARGET(str_printf)
{
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
const std::string format_string = fuzzed_data_provider.ConsumeRandomLengthString(64);
const bilingual_str bilingual_string{format_string, format_string};

const int digits_in_format_specifier = std::count_if(format_string.begin(), format_string.end(), IsDigit);

Expand Down Expand Up @@ -52,28 +57,22 @@ FUZZ_TARGET(str_printf)
CallOneOf(
fuzzed_data_provider,
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32));
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<signed char>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<char>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeBool());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeBool());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeBool());
});
} catch (const tinyformat::format_error&) {
}
Expand All @@ -98,36 +97,28 @@ FUZZ_TARGET(str_printf)
CallOneOf(
fuzzed_data_provider,
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeFloatingPoint<double>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint16_t>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint32_t>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
},
[&] {
(void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
(void)tinyformat::format(bilingual_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
});
} catch (const tinyformat::format_error&) {
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/fuzz/util/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ struct FuzzedWallet {

for (const std::string& desc_fmt : DESCS) {
for (bool internal : {true, false}) {
const auto descriptor{(strprintf)(desc_fmt, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})};
const auto descriptor{strprintf(tfm::RuntimeFormat{desc_fmt}, "[5aa9973a/66h/4h/2h]" + seed_insecure, int{internal})};

FlatSigningProvider keys;
std::string error;
Expand Down
19 changes: 16 additions & 3 deletions src/test/translation_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,26 @@

BOOST_AUTO_TEST_SUITE(translation_tests)

static TranslateFn translate{[](const char * str) { return strprintf("t(%s)", str); }};

// Custom translation function _t(), similar to _() but internal to this test.
consteval auto _t(util::TranslatedLiteral str)
{
str.translate_fn = &translate;
return str;
}

BOOST_AUTO_TEST_CASE(translation_namedparams)
{
bilingual_str arg{"original", "translated"};
bilingual_str format{"original [%s]", "translated [%s]"};
bilingual_str result{strprintf(format, arg)};
bilingual_str result{strprintf(_t("original [%s]"), arg)};
BOOST_CHECK_EQUAL(result.original, "original [original]");
BOOST_CHECK_EQUAL(result.translated, "translated [translated]");
BOOST_CHECK_EQUAL(result.translated, "t(original [translated])");

util::TranslatedLiteral arg2{"original", &translate};
bilingual_str result2{strprintf(_t("original [%s]"), arg2)};
BOOST_CHECK_EQUAL(result2.original, "original [original]");
BOOST_CHECK_EQUAL(result2.translated, "t(original [t(original)])");
}

BOOST_AUTO_TEST_SUITE_END()
2 changes: 1 addition & 1 deletion src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ using node::LoadChainstate;
using node::RegenerateCommitments;
using node::VerifyLoadedChainstate;

const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
const TranslateFn G_TRANSLATION_FUN{nullptr};

constexpr inline auto TEST_DIR_PATH_ELEMENT{"test_common bitcoin"}; // Includes a space to catch possible path escape issues.
/** Random context to get unique temp data dirs. Separate from m_rng, which can be seeded from a const env var */
Expand Down
2 changes: 1 addition & 1 deletion src/test/util_string_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ template <unsigned NumArgs>
void TfmFormatZeroes(const std::string& fmt)
{
std::apply([&](auto... args) {
(void)tfm::format(fmt, args...);
(void)tfm::format(tfm::RuntimeFormat{fmt}, args...);
}, std::array<int, NumArgs>{});
}

Expand Down
13 changes: 10 additions & 3 deletions src/tinyformat.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ namespace tfm = tinyformat;
//------------------------------------------------------------------------------
// Implementation details.
#include <algorithm>
#include <attributes.h> // Added for Bitcoin Core
#include <iostream>
#include <sstream>
#include <stdexcept> // Added for Bitcoin Core
Expand Down Expand Up @@ -179,13 +180,19 @@ namespace tfm = tinyformat;

namespace tinyformat {

// Added for Bitcoin Core. Similar to std::runtime_format from C++26.
struct RuntimeFormat {
const std::string& fmt; // Not a string view, because tinyformat requires a c_str
explicit RuntimeFormat(LIFETIMEBOUND const std::string& str) : fmt{str} {}
};

// Added for Bitcoin Core. Wrapper for checking format strings at compile time.
// Unlike ConstevalFormatString this supports std::string for runtime string
// formatting without compile time checks.
// Unlike ConstevalFormatString this supports RunTimeFormat-wrapped std::string
// for runtime string formatting without compile time checks.
template <unsigned num_params>
struct FormatStringCheck {
consteval FormatStringCheck(const char* str) : fmt{util::ConstevalFormatString<num_params>{str}.fmt} {}
FormatStringCheck(const std::string& str) : fmt{str.c_str()} {}
FormatStringCheck(LIFETIMEBOUND const RuntimeFormat& run) : fmt{run.fmt.c_str()} {}
FormatStringCheck(util::ConstevalFormatString<num_params> str) : fmt{str.fmt} {}
operator const char*() { return fmt; }
const char* fmt;
Expand Down
Loading
Loading