From 9ba513396764c58d17fd6270f9e63da987a72b1a Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 8 Jul 2023 10:37:44 +0800 Subject: [PATCH 1/3] treewide: s/format/seastar::format/ this change replace all `format()` with `seastar::format()`. this was not necessary if we don't change the function signature of `seastar::format()`, even if we compile the tree with libstdc++ shipped with GCC-13, which implements `std::format()`. because `std::format()` accepts a templated parameter of `fmt`, so the `seastar::fmt()` which accepts a plain `const char*` always win if we just call `format()` in the opened namespace of `seastar`. but since we plan to change `seastar::format()` from `format(const char* fmt, A&&... a)` to `format(fmt::format_string fmt, A&&... a)`, we cannot gurantee that `seastar::format()` will win in the function resolution in the use case above. so we have to change all call sites of this function to `seastar::format()` to disambiguate it. Signed-off-by: Kefu Chai --- apps/io_tester/io_tester.cc | 4 ++-- src/core/semaphore.cc | 6 +++--- src/json/formatter.cc | 2 +- src/net/net.cc | 14 +++++++------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/apps/io_tester/io_tester.cc b/apps/io_tester/io_tester.cc index 574e7a589fe..12f820ebf30 100644 --- a/apps/io_tester/io_tester.cc +++ b/apps/io_tester/io_tester.cc @@ -966,7 +966,7 @@ struct convert { } else if (st == "steady") { op.sleep_fn = timer_sleep; } else { - throw std::runtime_error(format("Unknown sleep_type {}", st)); + throw std::runtime_error(seastar::format("Unknown sleep_type {}", st)); } } if (node["pause_distribution"]) { @@ -976,7 +976,7 @@ struct convert { } else if (pd == "poisson") { op.pause_fn = make_poisson_pause; } else { - throw std::runtime_error(format("Unknown pause_distribution {}", pd)); + throw std::runtime_error(seastar::format("Unknown pause_distribution {}", pd)); } } return true; diff --git a/src/core/semaphore.cc b/src/core/semaphore.cc index 2f05edc92fe..233dc65298e 100644 --- a/src/core/semaphore.cc +++ b/src/core/semaphore.cc @@ -85,7 +85,7 @@ static_assert(std::is_nothrow_move_constructible_v); named_semaphore_timed_out::named_semaphore_timed_out(std::string_view msg) noexcept : _msg() { try { - _msg = format("Semaphore timed out: {}", msg); + _msg = seastar::format("Semaphore timed out: {}", msg); } catch (...) { // ignore, empty _msg will generate a static message in what(). } @@ -93,7 +93,7 @@ named_semaphore_timed_out::named_semaphore_timed_out(std::string_view msg) noexc broken_named_semaphore::broken_named_semaphore(std::string_view msg) noexcept : _msg() { try { - _msg = format("Semaphore broken: {}", msg); + _msg = seastar::format("Semaphore broken: {}", msg); } catch (...) { // ignore, empty _msg will generate a static message in what(). } @@ -101,7 +101,7 @@ broken_named_semaphore::broken_named_semaphore(std::string_view msg) noexcept : named_semaphore_aborted::named_semaphore_aborted(std::string_view msg) noexcept : _msg() { try { - _msg = format("Semaphore aborted: {}", msg); + _msg = seastar::format("Semaphore aborted: {}", msg); } catch (...) { // ignore, empty _msg will generate a static message in what(). } diff --git a/src/json/formatter.cc b/src/json/formatter.cc index 1616e29e412..de45d932a91 100644 --- a/src/json/formatter.cc +++ b/src/json/formatter.cc @@ -71,7 +71,7 @@ static bool needs_escaping(const string_view& str) { static sstring string_view_to_json(const string_view& str) { if (!needs_escaping(str)) { - return format("\"{}\"", str); + return seastar::format("\"{}\"", str); } ostringstream oss; diff --git a/src/net/net.cc b/src/net/net.cc index b75bc534107..ef722e2b78b 100644 --- a/src/net/net.cc +++ b/src/net/net.cc @@ -154,7 +154,7 @@ qp::qp(bool register_copy_stats, // // Tx sm::make_gauge(_queue_name + "_tx_packet_queue_last_bunch", _stats.tx.good.last_bunch, - sm::description(format("Holds a number of packets sent in the bunch. " + sm::description(seastar::format("Holds a number of packets sent in the bunch. " "A high value in conjunction with a high value of a {} indicates an efficient Tx packets bulking.", _queue_name + "_tx_packet_queue"))), // Rx sm::make_gauge(_queue_name + "_rx_packet_queue_last_bunch", _stats.rx.good.last_bunch, @@ -165,10 +165,10 @@ qp::qp(bool register_copy_stats, // // Tx sm::make_counter(_queue_name + "_tx_frags", _stats.tx.good.nr_frags, - sm::description(format("Counts a number of sent fragments. Divide this value by a {} to get an average number of fragments in a Tx packet.", _queue_name + "_tx_packets"))), + sm::description(seastar::format("Counts a number of sent fragments. Divide this value by a {} to get an average number of fragments in a Tx packet.", _queue_name + "_tx_packets"))), // Rx sm::make_counter(_queue_name + "_rx_frags", _stats.rx.good.nr_frags, - sm::description(format("Counts a number of received fragments. Divide this value by a {} to get an average number of fragments in an Rx packet.", _queue_name + "_rx_packets"))), + sm::description(seastar::format("Counts a number of received fragments. Divide this value by a {} to get an average number of fragments in an Rx packet.", _queue_name + "_rx_packets"))), }); if (register_copy_stats) { @@ -178,20 +178,20 @@ qp::qp(bool register_copy_stats, // // Tx sm::make_counter(_queue_name + "_tx_copy_bytes", _stats.tx.good.copy_bytes, - sm::description(format("Counts a number of sent bytes that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of data sent using a non-zero-copy flow.", _queue_name + "_tx_bytes"))), + sm::description(seastar::format("Counts a number of sent bytes that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of data sent using a non-zero-copy flow.", _queue_name + "_tx_bytes"))), // Rx sm::make_counter(_queue_name + "_rx_copy_bytes", _stats.rx.good.copy_bytes, - sm::description(format("Counts a number of received bytes that were handled in a non-zero-copy way. Divide this value by an {} to get a portion of received data handled using a non-zero-copy flow.", _queue_name + "_rx_bytes"))), + sm::description(seastar::format("Counts a number of received bytes that were handled in a non-zero-copy way. Divide this value by an {} to get a portion of received data handled using a non-zero-copy flow.", _queue_name + "_rx_bytes"))), // // Non-zero-copy data fragments rate: DERIVE:0:u // // Tx sm::make_counter(_queue_name + "_tx_copy_frags", _stats.tx.good.copy_frags, - sm::description(format("Counts a number of sent fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of fragments sent using a non-zero-copy flow.", _queue_name + "_tx_frags"))), + sm::description(seastar::format("Counts a number of sent fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of fragments sent using a non-zero-copy flow.", _queue_name + "_tx_frags"))), // Rx sm::make_counter(_queue_name + "_rx_copy_frags", _stats.rx.good.copy_frags, - sm::description(format("Counts a number of received fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of received fragments handled using a non-zero-copy flow.", _queue_name + "_rx_frags"))), + sm::description(seastar::format("Counts a number of received fragments that were handled in a non-zero-copy way. Divide this value by a {} to get a portion of received fragments handled using a non-zero-copy flow.", _queue_name + "_rx_frags"))), }); } From 769d1b42a7a7bbbd5061ac3c9c21755418ead013 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 8 Jul 2023 11:00:18 +0800 Subject: [PATCH 2/3] rpc: do not use seastar::format() in rpc logger because rpc logger needs to talk to both seastar::logger and the user customized logger. the former does not accept `fmt::format_string` yet, while the latter needs to accept `seastar::format()`ed sstring. and we plan to change `seastar::format()` to use compile-time format check. so to cater the needs of both branches. we just decouple rpc logger from `seastar::format()`. and let rpc logger to use `fmt::format_to()` directly. this would allow us to change `seastar::format()`. Signed-off-by: Kefu Chai --- include/seastar/rpc/rpc.hh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/seastar/rpc/rpc.hh b/include/seastar/rpc/rpc.hh index b4eecd6ade5..fc797213e2b 100644 --- a/include/seastar/rpc/rpc.hh +++ b/include/seastar/rpc/rpc.hh @@ -217,7 +217,9 @@ class logger { // Ignore less severe levels in order not to spam user's log with messages during transition, // i.e. when the user still only defines a level-less logger. } else if (_logger && level <= log_level::info) { - _logger(format(fmt, std::forward(args)...)); + fmt::memory_buffer out; + fmt::format_to(fmt::appender(out), fmt::runtime(fmt), std::forward(args)...); + _logger(sstring{out.data(), out.size()}); } } From b6015b40716269f3e242e24a863ce19510e26625 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 8 Jul 2023 11:04:47 +0800 Subject: [PATCH 3/3] print: use fmtlib's fmt::format_string in format() using `fmt::format_string` would allow us to perform compile-time format checking. so in this change, we switch to `fmt::format_string` when fmtlib's version is greater or equal to 8.0, which introduced the compile-time checking. also, because the `fmt::format_string` can only be instantiated with a compile-time constexpr, the fmt string specified in test is changed to a constexpr accordingly. Signed-off-by: Kefu Chai --- include/seastar/core/print.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/seastar/core/print.hh b/include/seastar/core/print.hh index 1576a0e1b85..5d276589650 100644 --- a/include/seastar/core/print.hh +++ b/include/seastar/core/print.hh @@ -140,9 +140,9 @@ log(A&&... a) { */ template sstring -format(const char* fmt, A&&... a) { +format(fmt::format_string fmt, A&&... a) { fmt::memory_buffer out; - fmt::format_to(fmt::appender(out), fmt::runtime(fmt), std::forward(a)...); + fmt::format_to(fmt::appender(out), fmt, std::forward(a)...); return sstring{out.data(), out.size()}; }