Skip to content

Commit b3c5b08

Browse files
elbenoelbeno
committed
🎨 Logging: allow writer in env, remove log_by_{args,buf}
Problem: - The writer has to be supplied in the log config; there are use cases for changing the writer in the environment. For instance this would provide an alternative, more flexible way to do secure logging. - The hardcoded dichotomy between `log_by_args` and `log_by_buf` is annoying and a relic of the original implementation. Solution: - Allow the log writer to be passed in the log environment. - Instead of `log_by_args` and `log_by_buf`, a writer should provide a function that takes the span to write. Now the callee can choose whether to make that a function template and potentially unroll the write loop, or just a function taking a dynamic span with a runtime loop.
1 parent 15ab5b2 commit b3c5b08

File tree

10 files changed

+210
-165
lines changed

10 files changed

+210
-165
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ include(cmake/string_catalog.cmake)
2525
add_versioned_package("gh:boostorg/mp11#boost-1.83.0")
2626
fmt_recipe(11.1.3)
2727
add_versioned_package("gh:intel/cpp-baremetal-concurrency#09c043b")
28-
add_versioned_package("gh:intel/cpp-std-extensions#8e2f948")
28+
add_versioned_package("gh:intel/cpp-std-extensions#ca9e67a")
2929
add_versioned_package("gh:intel/cpp-baremetal-senders-and-receivers#27db6e1")
3030

3131
set(GEN_STR_CATALOG

‎include/log/catalog/encoder.hpp‎

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <log/catalog/arguments.hpp>
44
#include <log/catalog/builder.hpp>
55
#include <log/catalog/catalog.hpp>
6+
#include <log/catalog/writer.hpp>
67
#include <log/log.hpp>
78
#include <log/module.hpp>
89
#include <log/module_id.hpp>
@@ -44,40 +45,23 @@ template <typename S, auto Id> struct to_message_t {
4445
template <typename... Args>
4546
using fn = decltype(to_message<S, Id, Args...>());
4647
};
48+
4749
} // namespace detail
4850

4951
template <typename Destinations> struct log_writer {
50-
template <std::size_t N>
51-
auto operator()(stdx::span<std::uint8_t const, N> msg) -> void {
52+
auto operator()(auto msg) -> void {
5253
stdx::for_each(
5354
[&]<typename Dest>(Dest &dest) {
54-
conc::call_in_critical_section<Dest>(
55-
[&] { dest.log_by_buf(msg); });
55+
conc::call_in_critical_section<Dest>([&] { dest(msg); });
5656
},
5757
dests);
5858
}
5959

60-
template <std::size_t N>
61-
auto operator()(stdx::span<std::uint32_t const, N> msg) -> void {
62-
[&]<std::size_t... Is>(std::index_sequence<Is...>) {
63-
stdx::for_each(
64-
[&]<typename Dest>(Dest &dest) {
65-
conc::call_in_critical_section<Dest>(
66-
[&] { dest.log_by_args(msg[Is]...); });
67-
},
68-
dests);
69-
}(std::make_index_sequence<N>{});
70-
}
71-
72-
auto operator()(auto const &msg) -> void {
73-
this->operator()(msg.as_const_view().data());
74-
}
75-
7660
Destinations dests;
7761
};
7862
template <typename T> log_writer(T) -> log_writer<T>;
7963

80-
template <typename Writer> struct log_handler {
64+
template <writer_like Writer> struct log_handler {
8165
template <typename Env, typename FilenameStringType,
8266
typename LineNumberType, typename FmtResult>
8367
auto log(FilenameStringType, LineNumberType, FmtResult const &fr) -> void {
@@ -86,8 +70,9 @@ template <typename Writer> struct log_handler {
8670

8771
template <typename Env, typename FmtResult>
8872
auto log_msg(FmtResult const &fr) -> void {
73+
auto builder = get_builder(Env{});
74+
writer_like auto writer = stdx::query<Env>(get_writer, w);
8975
fr.args.apply([&]<typename... Args>(Args &&...args) {
90-
auto builder = get_builder(Env{});
9176
constexpr auto L = stdx::to_underlying(get_level(Env{}));
9277
using Message = typename decltype(builder)::template convert_args<
9378
detail::to_message_t<decltype(fr.str), logging::get_string_id(
@@ -96,26 +81,31 @@ template <typename Writer> struct log_handler {
9681
using Module =
9782
decltype(detail::to_module<get_module(Env{}),
9883
logging::get_module_id(Env{})>());
99-
w(builder.template build<L>(catalog<Message>(), module<Module>(),
100-
std::forward<Args>(args)...));
84+
auto const pkt =
85+
builder.template build<L>(catalog<Message>(), module<Module>(),
86+
std::forward<Args>(args)...);
87+
writer(pkt.as_const_view().data());
10188
});
10289
}
10390

10491
template <typename Env, auto Version, stdx::ct_string S = "">
10592
auto log_version() -> void {
10693
auto builder = get_builder(Env{});
107-
w(builder.template build_version<Version, S>());
94+
writer_like auto writer = stdx::query<Env>(get_writer, w);
95+
auto const pkt = builder.template build_version<Version, S>();
96+
writer(pkt.as_const_view().data());
10897
}
10998

11099
Writer w;
111100
};
112101

113-
template <typename... TDestinations> struct config {
114-
using destinations_tuple_t = stdx::tuple<TDestinations...>;
115-
constexpr explicit config(TDestinations... dests)
102+
template <writer_like... Dests> struct config {
103+
using destinations_tuple_t = stdx::tuple<Dests...>;
104+
constexpr explicit config(Dests... dests)
116105
: logger{log_writer{stdx::tuple{std::move(dests)...}}} {}
117106

118107
log_handler<log_writer<destinations_tuple_t>> logger;
119108
};
109+
120110
template <typename... Ts> config(Ts...) -> config<Ts...>;
121111
} // namespace logging::binary

‎include/log/catalog/mipi_builder.hpp‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,13 @@ template <packer P> struct builder<defn::catalog_msg_t, P> {
6969
using namespace msg;
7070
constexpr auto payload_size =
7171
(sizeof(id) + ... + sizeof(typename P::template pack_as_t<Ts>));
72-
if constexpr (payload_size <= sizeof(uint32_t) * 3) {
73-
constexpr auto header_size =
74-
defn::catalog_msg_t::size<std::uint32_t>::value;
75-
using storage_t =
76-
std::array<std::uint32_t,
77-
header_size +
78-
stdx::sized8{payload_size}.in<std::uint32_t>()>;
79-
return catalog_builder<storage_t, P>{}.template build<Level>(
80-
id, m, args...);
81-
} else {
82-
constexpr auto header_size =
83-
defn::catalog_msg_t::size<std::uint8_t>::value;
84-
using storage_t =
85-
std::array<std::uint8_t, header_size + payload_size>;
86-
return catalog_builder<storage_t, P>{}.template build<Level>(
87-
id, m, args...);
88-
}
72+
constexpr auto header_size =
73+
defn::catalog_msg_t::size<std::uint32_t>::value;
74+
using storage_t =
75+
std::array<std::uint32_t, header_size + stdx::sized8{payload_size}
76+
.in<std::uint32_t>()>;
77+
return catalog_builder<storage_t, P>{}.template build<Level>(id, m,
78+
args...);
8979
}
9080
};
9181

‎include/log/catalog/writer.hpp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#pragma once
2+
3+
#include <log/catalog/mipi_builder.hpp>
4+
5+
#include <stdx/compiler.hpp>
6+
#include <stdx/span.hpp>
7+
8+
#include <cstdint>
9+
#include <utility>
10+
11+
namespace logging::binary {
12+
template <typename T>
13+
concept writer_like =
14+
requires(T &t, stdx::span<std::uint32_t const, 1> data) { t(data); };
15+
16+
[[maybe_unused]] constexpr inline struct get_writer_t {
17+
template <typename T>
18+
requires true
19+
CONSTEVAL auto operator()(T &&t) const noexcept(
20+
noexcept(std::forward<T>(t).query(std::declval<get_writer_t>())))
21+
-> decltype(std::forward<T>(t).query(*this)) {
22+
return std::forward<T>(t).query(*this);
23+
}
24+
} get_writer;
25+
} // namespace logging::binary

‎test/log/catalog1_lib.cpp‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <stdx/ct_format.hpp>
66
#include <stdx/ct_string.hpp>
7+
#include <stdx/span.hpp>
78

89
#include <conc/concurrency.hpp>
910

@@ -15,10 +16,11 @@ int log_calls{};
1516
std::uint32_t last_header{};
1617

1718
namespace {
18-
struct test_log_args_destination {
19-
auto log_by_args(std::uint32_t hdr, auto...) -> void {
19+
struct test_log_destination {
20+
template <std::size_t N>
21+
auto operator()(stdx::span<std::uint32_t const, N> pkt) const {
2022
++log_calls;
21-
last_header = hdr;
23+
last_header = pkt[0];
2224
}
2325
};
2426

@@ -36,40 +38,40 @@ auto log_with_fixed_string_id() -> void;
3638
auto log_with_fixed_module_id() -> void;
3739

3840
auto log_zero_args() -> void {
39-
auto cfg = logging::binary::config{test_log_args_destination{}};
41+
auto cfg = logging::binary::config{test_log_destination{}};
4042
cfg.logger.log_msg<log_env1>(
4143
stdx::ct_format<"A string with no placeholders">());
4244
}
4345

4446
auto log_one_ct_arg() -> void {
4547
using namespace stdx::literals;
46-
auto cfg = logging::binary::config{test_log_args_destination{}};
48+
auto cfg = logging::binary::config{test_log_destination{}};
4749
cfg.logger.log_msg<log_env1>(
4850
stdx::ct_format<"B string with {} placeholder">("one"_ctst));
4951
}
5052

5153
auto log_one_32bit_rt_arg() -> void {
52-
auto cfg = logging::binary::config{test_log_args_destination{}};
54+
auto cfg = logging::binary::config{test_log_destination{}};
5355
cfg.logger.log_msg<log_env1>(
5456
stdx::ct_format<"C1 string with {} placeholder">(std::int32_t{1}));
5557
}
5658

5759
auto log_one_64bit_rt_arg() -> void {
58-
auto cfg = logging::binary::config{test_log_args_destination{}};
60+
auto cfg = logging::binary::config{test_log_destination{}};
5961
cfg.logger.log_msg<log_env1>(
6062
stdx::ct_format<"C2 string with {} placeholder">(std::int64_t{1}));
6163
}
6264

6365
auto log_one_formatted_rt_arg() -> void {
64-
auto cfg = logging::binary::config{test_log_args_destination{}};
66+
auto cfg = logging::binary::config{test_log_destination{}};
6567
cfg.logger.log_msg<log_env1>(
6668
stdx::ct_format<"C3 string with {:08x} placeholder">(std::int32_t{1}));
6769
}
6870

6971
auto log_with_non_default_module() -> void {
7072
CIB_WITH_LOG_ENV(logging::get_level, logging::level::TRACE,
7173
logging::get_module, "not default") {
72-
auto cfg = logging::binary::config{test_log_args_destination{}};
74+
auto cfg = logging::binary::config{test_log_destination{}};
7375
cfg.logger.log_msg<cib_log_env_t>(
7476
stdx::ct_format<"ModuleID string with {} placeholder">(1));
7577
}
@@ -78,7 +80,7 @@ auto log_with_non_default_module() -> void {
7880
auto log_with_fixed_module() -> void {
7981
CIB_WITH_LOG_ENV(logging::get_level, logging::level::TRACE,
8082
logging::get_module, "fixed") {
81-
auto cfg = logging::binary::config{test_log_args_destination{}};
83+
auto cfg = logging::binary::config{test_log_destination{}};
8284
cfg.logger.log_msg<cib_log_env_t>(
8385
stdx::ct_format<"Fixed ModuleID string with {} placeholder">(1));
8486
}
@@ -87,7 +89,7 @@ auto log_with_fixed_module() -> void {
8789
auto log_with_fixed_string_id() -> void {
8890
CIB_WITH_LOG_ENV(logging::get_level, logging::level::TRACE,
8991
logging::get_string_id, 1337) {
90-
auto cfg = logging::binary::config{test_log_args_destination{}};
92+
auto cfg = logging::binary::config{test_log_destination{}};
9193
cfg.logger.log_msg<cib_log_env_t>(
9294
stdx::ct_format<"Fixed StringID string">());
9395
}
@@ -97,7 +99,7 @@ auto log_with_fixed_module_id() -> void {
9799
CIB_WITH_LOG_ENV(logging::get_level, logging::level::TRACE,
98100
logging::get_module_id, 7, logging::get_module,
99101
"fixed_id") {
100-
auto cfg = logging::binary::config{test_log_args_destination{}};
102+
auto cfg = logging::binary::config{test_log_destination{}};
101103
cfg.logger.log_msg<cib_log_env_t>(
102104
stdx::ct_format<"Fixed ModuleID string with {} placeholder">(1));
103105
}

‎test/log/catalog2a_lib.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <log/catalog/encoder.hpp>
55

66
#include <stdx/ct_format.hpp>
7+
#include <stdx/span.hpp>
78

89
#include <conc/concurrency.hpp>
910

@@ -14,10 +15,9 @@ template <> inline auto conc::injected_policy<> = test_conc_policy{};
1415
extern int log_calls;
1516

1617
namespace {
17-
struct test_log_args_destination {
18-
auto log_by_args(std::uint32_t, auto...) -> void { ++log_calls; }
18+
struct test_log_destination {
1919
template <std::size_t N>
20-
auto log_by_buf(stdx::span<std::uint8_t const, N>) const {
20+
auto operator()(stdx::span<std::uint32_t const, N>) const {
2121
++log_calls;
2222
}
2323
};
@@ -28,7 +28,7 @@ using log_env2a = stdx::make_env_t<logging::get_level, logging::level::TRACE>;
2828
auto log_two_rt_args() -> void;
2929

3030
auto log_two_rt_args() -> void {
31-
auto cfg = logging::binary::config{test_log_args_destination{}};
31+
auto cfg = logging::binary::config{test_log_destination{}};
3232
cfg.logger.log_msg<log_env2a>(
3333
stdx::ct_format<"D string with {} and {} placeholder">(
3434
std::uint32_t{1}, std::int64_t{2}));

‎test/log/catalog2b_lib.cpp‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <log/catalog/encoder.hpp>
55

66
#include <stdx/ct_format.hpp>
7+
#include <stdx/span.hpp>
78

89
#include <conc/concurrency.hpp>
910

@@ -14,8 +15,11 @@ template <> inline auto conc::injected_policy<> = test_conc_policy{};
1415
extern int log_calls;
1516

1617
namespace {
17-
struct test_log_args_destination {
18-
auto log_by_args(std::uint32_t, auto...) -> void { ++log_calls; }
18+
struct test_log_destination {
19+
template <std::size_t N>
20+
auto operator()(stdx::span<std::uint32_t const, N>) const {
21+
++log_calls;
22+
}
1923
};
2024

2125
using log_env2b = stdx::make_env_t<logging::get_level, logging::level::TRACE>;
@@ -26,21 +30,21 @@ auto log_rt_float_arg() -> void;
2630
auto log_rt_double_arg() -> void;
2731

2832
auto log_rt_enum_arg() -> void {
29-
auto cfg = logging::binary::config{test_log_args_destination{}};
33+
auto cfg = logging::binary::config{test_log_destination{}};
3034
using namespace ns;
3135
cfg.logger.log_msg<log_env2b>(
3236
stdx::ct_format<"E string with {} placeholder">(E::value));
3337
}
3438

3539
auto log_rt_float_arg() -> void {
36-
auto cfg = logging::binary::config{test_log_args_destination{}};
40+
auto cfg = logging::binary::config{test_log_destination{}};
3741
using namespace ns;
3842
cfg.logger.log_msg<log_env2b>(
3943
stdx::ct_format<"Float string with {} placeholder">(3.14f));
4044
}
4145

4246
auto log_rt_double_arg() -> void {
43-
auto cfg = logging::binary::config{test_log_args_destination{}};
47+
auto cfg = logging::binary::config{test_log_destination{}};
4448
using namespace ns;
4549
cfg.logger.log_msg<log_env2b>(
4650
stdx::ct_format<"Double string with {} placeholder">(3.14));

0 commit comments

Comments
 (0)