Skip to content

Commit 184f34f

Browse files
ryanofskyMarcoFalkehodlinatorl0rinc
committed
util: Support dynamic width & precision in ConstevalFormatString
This is needed in the next commit to add compile-time checking to strprintf calls, because bitcoin-cli.cpp uses dynamic width in many format strings. This change is easiest to review ignoring whitespace. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^[email protected]> Co-authored-by: Hodlinator <[email protected]> Co-authored-by: l0rinc <[email protected]>
1 parent da10e0b commit 184f34f

File tree

2 files changed

+82
-40
lines changed

2 files changed

+82
-40
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/util/string.h

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,53 +25,65 @@ namespace util {
2525
* strings, to reduce the likelihood of tinyformat throwing exceptions at
2626
* run-time. Validation is partial to try and prevent the most common errors
2727
* 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.
3428
*/
3529
template <unsigned num_params>
3630
struct ConstevalFormatString {
3731
const char* const fmt;
3832
consteval ConstevalFormatString(const char* str) : fmt{str} { Detail_CheckNumFormatSpecifiers(fmt); }
39-
constexpr static void Detail_CheckNumFormatSpecifiers(std::string_view str)
33+
constexpr static void Detail_CheckNumFormatSpecifiers(const char* str)
4034
{
4135
unsigned count_normal{0}; // Number of "normal" specifiers, like %s
4236
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-
}
37+
for (auto it{str}; *it != '\0'; ++it) {
38+
if (*it != '%' || *++it == '%') continue; // Skip escaped %%
4839

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-
}
40+
auto add_arg = [&] {
41+
unsigned maybe_num{0};
42+
while ('0' <= *it && *it <= '9') {
43+
maybe_num *= 10;
44+
maybe_num += *it - '0';
45+
++it;
46+
}
5547

56-
unsigned maybe_num{0};
57-
while ('0' <= *it && *it <= '9') {
58-
maybe_num *= 10;
59-
maybe_num += *it - '0';
60-
++it;
48+
if (*it == '$') {
49+
++it;
50+
// Positional specifier, like %8$s
51+
if (maybe_num == 0) throw "Positional format specifier must have position of at least 1";
52+
count_pos = std::max(count_pos, maybe_num);
53+
} else {
54+
// Non-positional specifier, like %s
55+
++count_normal;
56+
}
6157
};
6258

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;
59+
// Increase argument count and consume positional specifier, if present.
60+
add_arg();
61+
62+
// Consume flags.
63+
while (*it == '#' || *it == '0' || *it == '-' || *it == ' ' || *it == '+') ++it;
64+
65+
auto parse_size = [&] {
66+
if (*it == '*') {
67+
++it;
68+
add_arg();
69+
} else {
70+
while ('0' <= *it && *it <= '9') ++it;
71+
}
72+
};
73+
74+
// Consume dynamic or static width value.
75+
parse_size();
76+
77+
// Consume dynamic or static precision value.
78+
if (*it == '.') {
7179
++it;
80+
parse_size();
7281
}
73-
// The remainder "[flags][width][.precision][length]type" of the
74-
// specifier is not checked. Parsing continues with the next '%'.
82+
83+
if (*it == '\0') throw "Format specifier incorrectly terminated by end of string";
84+
85+
// Length and type in "[flags][width][.precision][length]type"
86+
// is not checked. Parsing continues with the next '%'.
7587
}
7688
if (count_normal && count_pos) throw "Format specifiers must be all positional or all non-positional!";
7789
unsigned count{count_normal | count_pos};

0 commit comments

Comments
 (0)