Skip to content

Commit f44e39c

Browse files
fanquakefanquake
committed
Merge #31174: tinyformat: Add compile-time checking for literal format strings
fe39acf tinyformat: Add compile-time checking for literal format strings (Ryan Ofsky) 184f34f util: Support dynamic width & precision in ConstevalFormatString (Ryan Ofsky) Pull request description: Add compile-time checking for literal format strings passed to `strprintf` and `tfm::format` to make sure the right number of format arguments are passed. There is still no compile-time checking if non-literal `std::string` or `bilingual_str` format strings are passed, but this is improved in other PRs: - [#31061](#31061) implements compile-time checking for translated strings - [#31072](#31072) increases compile-time checking by using literal strings as format strings, instead of `std::string` and `bilingual_str` - [#31149](#31149) may drop the `std::string` overload for `strprintf` to [require](#31149 (comment)) compile-time checking ACKs for top commit: maflcko: re-ACK fe39acf 🕐 l0rinc: ACK fe39acf hodlinator: re-ACK fe39acf Tree-SHA512: f1ddef0c96b9468c5ffe31b42dc19f1922583dd14f2e180b618d992c98614c5cc7db9f9cd917ef503f833bbc7dbec78e4489d0035416dce6840837e1d66d87cb
2 parents 69c0313 + fe39acf commit f44e39c

File tree

3 files changed

+99
-62
lines changed

3 files changed

+99
-62
lines changed

‎src/test/util_string_tests.cpp

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ inline void PassFmt(util::ConstevalFormatString<NumArgs> fmt)
2020
decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt);
2121
}
2222
template <unsigned WrongNumArgs>
23-
inline void FailFmtWithError(std::string_view wrong_fmt, std::string_view error)
23+
inline void FailFmtWithError(const char* wrong_fmt, std::string_view error)
2424
{
2525
BOOST_CHECK_EXCEPTION(util::ConstevalFormatString<WrongNumArgs>::Detail_CheckNumFormatSpecifiers(wrong_fmt), const char*, HasReason(error));
2626
}
@@ -44,6 +44,8 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
4444
PassFmt<1>("%+2s");
4545
PassFmt<1>("%.6i");
4646
PassFmt<1>("%5.2f");
47+
PassFmt<1>("%5.f");
48+
PassFmt<1>("%.f");
4749
PassFmt<1>("%#x");
4850
PassFmt<1>("%1$5i");
4951
PassFmt<1>("%1$-5i");
@@ -54,32 +56,60 @@ BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec)
5456
PassFmt<1>("%_");
5557
PassFmt<1>("%\n");
5658

57-
// The `*` specifier behavior is unsupported and can lead to runtime
58-
// errors when used in a ConstevalFormatString. Please refer to the
59-
// note in the ConstevalFormatString docs.
60-
PassFmt<1>("%*c");
61-
PassFmt<2>("%2$*3$d");
62-
PassFmt<1>("%.*f");
59+
PassFmt<2>("%*c");
60+
PassFmt<2>("%+*c");
61+
PassFmt<2>("%.*f");
62+
PassFmt<3>("%*.*f");
63+
PassFmt<3>("%2$*3$d");
64+
PassFmt<3>("%2$*3$.9d");
65+
PassFmt<3>("%2$.*3$d");
66+
PassFmt<3>("%2$9.*3$d");
67+
PassFmt<3>("%2$+9.*3$d");
68+
PassFmt<4>("%3$*2$.*4$f");
69+
70+
// Make sure multiple flag characters "- 0+" are accepted
71+
PassFmt<3>("'%- 0+*.*f'");
72+
PassFmt<3>("'%1$- 0+*3$.*2$f'");
6373

6474
auto err_mix{"Format specifiers must be all positional or all non-positional!"};
6575
FailFmtWithError<1>("%s%1$s", err_mix);
76+
FailFmtWithError<2>("%2$*d", err_mix);
77+
FailFmtWithError<2>("%*2$d", err_mix);
78+
FailFmtWithError<2>("%.*3$d", err_mix);
79+
FailFmtWithError<2>("%2$.*d", err_mix);
6680

6781
auto err_num{"Format specifier count must match the argument count!"};
6882
FailFmtWithError<1>("", err_num);
6983
FailFmtWithError<0>("%s", err_num);
7084
FailFmtWithError<2>("%s", err_num);
7185
FailFmtWithError<0>("%1$s", err_num);
7286
FailFmtWithError<2>("%1$s", err_num);
87+
FailFmtWithError<1>("%*c", err_num);
7388

7489
auto err_0_pos{"Positional format specifier must have position of at least 1"};
7590
FailFmtWithError<1>("%$s", err_0_pos);
7691
FailFmtWithError<1>("%$", err_0_pos);
7792
FailFmtWithError<0>("%0$", err_0_pos);
7893
FailFmtWithError<0>("%0$s", err_0_pos);
94+
FailFmtWithError<2>("%2$*$d", err_0_pos);
95+
FailFmtWithError<2>("%2$*0$d", err_0_pos);
96+
FailFmtWithError<3>("%3$*2$.*$f", err_0_pos);
97+
FailFmtWithError<3>("%3$*2$.*0$f", err_0_pos);
7998

8099
auto err_term{"Format specifier incorrectly terminated by end of string"};
81100
FailFmtWithError<1>("%", err_term);
101+
FailFmtWithError<1>("%9", err_term);
102+
FailFmtWithError<1>("%9.", err_term);
103+
FailFmtWithError<1>("%9.9", err_term);
104+
FailFmtWithError<1>("%*", err_term);
105+
FailFmtWithError<1>("%+*", err_term);
106+
FailFmtWithError<1>("%.*", err_term);
107+
FailFmtWithError<1>("%9.*", err_term);
82108
FailFmtWithError<1>("%1$", err_term);
109+
FailFmtWithError<1>("%1$9", err_term);
110+
FailFmtWithError<2>("%1$*2$", err_term);
111+
FailFmtWithError<2>("%1$.*2$", err_term);
112+
FailFmtWithError<2>("%1$9.*2$", err_term);
83113
}
84114

85115
BOOST_AUTO_TEST_SUITE_END()

‎src/tinyformat.h

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ namespace tfm = tinyformat;
145145
#include <iostream>
146146
#include <sstream>
147147
#include <stdexcept> // Added for Bitcoin Core
148+
#include <util/string.h> // Added for Bitcoin Core
148149

149150
#ifndef TINYFORMAT_ASSERT
150151
# include <cassert>
@@ -178,6 +179,18 @@ namespace tfm = tinyformat;
178179

179180
namespace tinyformat {
180181

182+
// Added for Bitcoin Core. Wrapper for checking format strings at compile time.
183+
// Unlike ConstevalFormatString this supports std::string for runtime string
184+
// formatting without compile time checks.
185+
template <unsigned num_params>
186+
struct FormatStringCheck {
187+
consteval FormatStringCheck(const char* str) : fmt{util::ConstevalFormatString<num_params>{str}.fmt} {}
188+
FormatStringCheck(const std::string& str) : fmt{str.c_str()} {}
189+
FormatStringCheck(util::ConstevalFormatString<num_params> str) : fmt{str.fmt} {}
190+
operator const char*() { return fmt; }
191+
const char* fmt;
192+
};
193+
181194
// Added for Bitcoin Core
182195
class format_error: public std::runtime_error
183196
{
@@ -1056,15 +1069,15 @@ inline void vformat(std::ostream& out, const char* fmt, FormatListRef list)
10561069

10571070
/// Format list of arguments to the stream according to given format string.
10581071
template<typename... Args>
1059-
void format(std::ostream& out, const char* fmt, const Args&... args)
1072+
void format(std::ostream& out, FormatStringCheck<sizeof...(Args)> fmt, const Args&... args)
10601073
{
10611074
vformat(out, fmt, makeFormatList(args...));
10621075
}
10631076

10641077
/// Format list of arguments according to the given format string and return
10651078
/// the result as a string.
10661079
template<typename... Args>
1067-
std::string format(const char* fmt, const Args&... args)
1080+
std::string format(FormatStringCheck<sizeof...(Args)> fmt, const Args&... args)
10681081
{
10691082
std::ostringstream oss;
10701083
format(oss, fmt, args...);
@@ -1073,13 +1086,13 @@ std::string format(const char* fmt, const Args&... args)
10731086

10741087
/// Format list of arguments to std::cout, according to the given format string
10751088
template<typename... Args>
1076-
void printf(const char* fmt, const Args&... args)
1089+
void printf(FormatStringCheck<sizeof...(Args)> fmt, const Args&... args)
10771090
{
10781091
format(std::cout, fmt, args...);
10791092
}
10801093

10811094
template<typename... Args>
1082-
void printfln(const char* fmt, const Args&... args)
1095+
void printfln(FormatStringCheck<sizeof...(Args)> fmt, const Args&... args)
10831096
{
10841097
format(std::cout, fmt, args...);
10851098
std::cout << '\n';
@@ -1145,15 +1158,6 @@ TINYFORMAT_FOREACH_ARGNUM(TINYFORMAT_MAKE_FORMAT_FUNCS)
11451158

11461159
#endif
11471160

1148-
// Added for Bitcoin Core
1149-
template<typename... Args>
1150-
std::string format(const std::string &fmt, const Args&... args)
1151-
{
1152-
std::ostringstream oss;
1153-
format(oss, fmt.c_str(), args...);
1154-
return oss.str();
1155-
}
1156-
11571161
} // namespace tinyformat
11581162

11591163
// Added for Bitcoin Core:

‎src/util/string.h

Lines changed: 45 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#define BITCOIN_UTIL_STRING_H
77

88
#include <span.h>
9-
#include <tinyformat.h>
109

1110
#include <array>
1211
#include <cstdint>
@@ -25,53 +24,65 @@ namespace util {
2524
* strings, to reduce the likelihood of tinyformat throwing exceptions at
2625
* run-time. Validation is partial to try and prevent the most common errors
2726
* while avoiding re-implementing the entire parsing logic.
28-
*
29-
* @note Counting of `*` dynamic width and precision fields (such as `%*c`,
30-
* `%2$*3$d`, `%.*f`) is not implemented to minimize code complexity as long as
31-
* they are not used in the codebase. Usage of these fields is not counted and
32-
* can lead to run-time exceptions. Code wanting to use the `*` specifier can
33-
* side-step this struct and call tinyformat directly.
3427
*/
3528
template <unsigned num_params>
3629
struct ConstevalFormatString {
3730
const char* const fmt;
3831
consteval ConstevalFormatString(const char* str) : fmt{str} { Detail_CheckNumFormatSpecifiers(fmt); }
39-
constexpr static void Detail_CheckNumFormatSpecifiers(std::string_view str)
32+
constexpr static void Detail_CheckNumFormatSpecifiers(const char* str)
4033
{
4134
unsigned count_normal{0}; // Number of "normal" specifiers, like %s
4235
unsigned count_pos{0}; // Max number in positional specifier, like %8$s
43-
for (auto it{str.begin()}; it < str.end();) {
44-
if (*it != '%') {
45-
++it;
46-
continue;
47-
}
36+
for (auto it{str}; *it != '\0'; ++it) {
37+
if (*it != '%' || *++it == '%') continue; // Skip escaped %%
38+
39+
auto add_arg = [&] {
40+
unsigned maybe_num{0};
41+
while ('0' <= *it && *it <= '9') {
42+
maybe_num *= 10;
43+
maybe_num += *it - '0';
44+
++it;
45+
}
46+
47+
if (*it == '$') {
48+
++it;
49+
// Positional specifier, like %8$s
50+
if (maybe_num == 0) throw "Positional format specifier must have position of at least 1";
51+
count_pos = std::max(count_pos, maybe_num);
52+
} else {
53+
// Non-positional specifier, like %s
54+
++count_normal;
55+
}
56+
};
4857

49-
if (++it >= str.end()) throw "Format specifier incorrectly terminated by end of string";
50-
if (*it == '%') {
51-
// Percent escape: %%
52-
++it;
53-
continue;
54-
}
58+
// Increase argument count and consume positional specifier, if present.
59+
add_arg();
5560

56-
unsigned maybe_num{0};
57-
while ('0' <= *it && *it <= '9') {
58-
maybe_num *= 10;
59-
maybe_num += *it - '0';
60-
++it;
61+
// Consume flags.
62+
while (*it == '#' || *it == '0' || *it == '-' || *it == ' ' || *it == '+') ++it;
63+
64+
auto parse_size = [&] {
65+
if (*it == '*') {
66+
++it;
67+
add_arg();
68+
} else {
69+
while ('0' <= *it && *it <= '9') ++it;
70+
}
6171
};
6272

63-
if (*it == '$') {
64-
// Positional specifier, like %8$s
65-
if (maybe_num == 0) throw "Positional format specifier must have position of at least 1";
66-
count_pos = std::max(count_pos, maybe_num);
67-
if (++it >= str.end()) throw "Format specifier incorrectly terminated by end of string";
68-
} else {
69-
// Non-positional specifier, like %s
70-
++count_normal;
73+
// Consume dynamic or static width value.
74+
parse_size();
75+
76+
// Consume dynamic or static precision value.
77+
if (*it == '.') {
7178
++it;
79+
parse_size();
7280
}
73-
// The remainder "[flags][width][.precision][length]type" of the
74-
// specifier is not checked. Parsing continues with the next '%'.
81+
82+
if (*it == '\0') throw "Format specifier incorrectly terminated by end of string";
83+
84+
// Length and type in "[flags][width][.precision][length]type"
85+
// is not checked. Parsing continues with the next '%'.
7586
}
7687
if (count_normal && count_pos) throw "Format specifiers must be all positional or all non-positional!";
7788
unsigned count{count_normal | count_pos};
@@ -235,12 +246,4 @@ template <typename T1, size_t PREFIX_LEN>
235246
}
236247
} // namespace util
237248

238-
namespace tinyformat {
239-
template <typename... Args>
240-
std::string format(util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
241-
{
242-
return format(fmt.fmt, args...);
243-
}
244-
} // namespace tinyformat
245-
246249
#endif // BITCOIN_UTIL_STRING_H

0 commit comments

Comments
 (0)