refactor: Introduce struct to hold a runtime format string

This brings the format types closer to the standard library types: * FormatStringCheck corresponds to std::basic_format_string, with compile-time checks done via ConstevalFormatString * RuntimeFormat corresponds to std::runtime_format, with no compile-time checks done. Also, it documents where no compile-time checks are done.
pablomartin4btc · Jan 15, 2025 · fa3efb5 · fa3efb5
1 parent fa6adb0
commit fa3efb5
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 20 deletions.
diff --git a/src/test/fuzz/strprintf.cpp b/src/test/fuzz/strprintf.cpp
@@ -14,6 +14,12 @@
 #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());
@@ -51,22 +57,22 @@ FUZZ_TARGET(str_printf)
         CallOneOf(
             fuzzed_data_provider,
             [&] {
-                (void)strprintf(format_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());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeRandomLengthString(32).c_str());
             },
             [&] {
-                (void)strprintf(format_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>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<unsigned char>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<char>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeBool());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeBool());
             });
     } catch (const tinyformat::format_error&) {
     }
@@ -91,28 +97,28 @@ FUZZ_TARGET(str_printf)
         CallOneOf(
             fuzzed_data_provider,
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeFloatingPoint<float>());
             },
             [&] {
-                (void)strprintf(format_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>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int16_t>());
             },
             [&] {
-                (void)strprintf(format_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>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int32_t>());
             },
             [&] {
-                (void)strprintf(format_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>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<int64_t>());
             },
             [&] {
-                (void)strprintf(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
+                fuzz_fmt(format_string, fuzzed_data_provider.ConsumeIntegral<uint64_t>());
             });
     } catch (const tinyformat::format_error&) {
     }

diff --git a/src/test/fuzz/util/wallet.h b/src/test/fuzz/util/wallet.h
@@ -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;

diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp
@@ -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>{});
 }
 

diff --git a/src/tinyformat.h b/src/tinyformat.h
@@ -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
@@ -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;

diff --git a/src/util/translation.h b/src/util/translation.h
@@ -103,7 +103,7 @@ bilingual_str format(util::BilingualFmt<sizeof...(Args)> fmt, const Args&... arg
         }
     }};
     return bilingual_str{tfm::format(fmt.original, original_arg(args)...),
-                         tfm::format(std::string{fmt.lit}, translated_arg(args)...)};
+                         tfm::format(RuntimeFormat{std::string{fmt.lit}}, translated_arg(args)...)};
 }
 } // namespace tinyformat