Skip to content

Commit 77c87a8

Browse files
elbenoelbeno
committed
🐛 Fix return value of indexed_handler::handle
An indexed handler may have indexed callbacks to call, but those callbacks may not handle the message, because they have extra matchers that weren't part of the index.
1 parent 6367d04 commit 77c87a8

File tree

4 files changed

+63
-33
lines changed

4 files changed

+63
-33
lines changed

‎include/msg/detail/indexed_builder_common.hpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,12 +141,12 @@ struct indexed_builder_base {
141141
ExtraCallbackArgsT...>{new_callbacks};
142142
}
143143

144-
using callback_func_t = void (*)(BaseMsgT const &,
145-
ExtraCallbackArgsT... args);
144+
using callback_func_t = auto (*)(BaseMsgT const &,
145+
ExtraCallbackArgsT... args) -> bool;
146146

147147
template <typename BuilderValue, std::size_t I>
148148
constexpr static auto invoke_callback(BaseMsgT const &data,
149-
ExtraCallbackArgsT... args) {
149+
ExtraCallbackArgsT... args) -> bool {
150150
// FIXME: incomplete message callback invocation...
151151
// 1) bit_cast message argument
152152
constexpr auto cb = IndexSpec{}.apply([&]<typename... Indices>(
@@ -173,7 +173,9 @@ struct indexed_builder_base {
173173
stdx::ct_string_to_type<cb.name, sc::string_constant>(),
174174
orig_cb.matcher.describe(), cb.matcher.describe());
175175
cb.callable(view, args...);
176+
return true;
176177
}
178+
return false;
177179
}
178180

179181
template <typename BuilderValue, std::size_t... Is>

‎include/msg/detail/indexed_handler_common.hpp‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ constexpr callback_args_t<BaseMsgT, ExtraCallbackArgsT...> callback_args{};
3232
template <typename IndexT, typename CallbacksT, typename BaseMsgT,
3333
typename... ExtraCallbackArgsT>
3434
struct indexed_handler : handler_interface<BaseMsgT, ExtraCallbackArgsT...> {
35-
using callback_func_t = void (*)(BaseMsgT const &,
36-
ExtraCallbackArgsT... args);
37-
3835
IndexT index;
3936
CallbacksT callback_entries;
4037

@@ -52,16 +49,14 @@ struct indexed_handler : handler_interface<BaseMsgT, ExtraCallbackArgsT...> {
5249
ExtraCallbackArgsT... args) const -> bool final {
5350
auto const callback_candidates = index(msg);
5451

55-
for_each([&](auto i) { callback_entries[i](msg, args...); },
52+
bool handled{};
53+
for_each([&](auto i) { handled |= callback_entries[i](msg, args...); },
5654
callback_candidates);
5755

58-
bool const candidates_found = !callback_candidates.none();
59-
60-
if (not candidates_found) {
56+
if (not handled) {
6157
CIB_ERROR("None of the registered callbacks claimed this message.");
6258
}
63-
64-
return candidates_found;
59+
return handled;
6560
}
6661
};
6762

‎test/msg/indexed_builder.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ TEST_CASE("match output success", "[handler_builder]") {
6868
cib::nexus<test_project> test_nexus{};
6969
test_nexus.init();
7070

71-
cib::service<test_service>->handle(
72-
test_msg_t{"test_id_field"_field = 0x80});
71+
CHECK(cib::service<test_service>->handle(
72+
test_msg_t{"test_id_field"_field = 0x80}));
7373
CAPTURE(log_buffer);
7474
CHECK(log_buffer.find("Incoming message matched") != std::string::npos);
7575
CHECK(log_buffer.find("[TestCallback]") != std::string::npos);
@@ -81,8 +81,8 @@ TEST_CASE("match output failure", "[handler_builder]") {
8181
cib::nexus<test_project> test_nexus{};
8282
test_nexus.init();
8383

84-
cib::service<test_service>->handle(
85-
test_msg_t{"test_id_field"_field = 0x81});
84+
CHECK(not cib::service<test_service>->handle(
85+
test_msg_t{"test_id_field"_field = 0x81}));
8686
CHECK(log_buffer.find(
8787
"None of the registered callbacks claimed this message") !=
8888
std::string::npos);
@@ -189,8 +189,8 @@ TEST_CASE("message matching partial index but not callback matcher",
189189

190190
log_buffer.clear();
191191
callback_success = false;
192-
cib::service<partially_indexed_test_service>->handle(test_msg_t{
193-
"test_id_field"_field = 0x80, "test_opcode_field"_field = 2});
192+
CHECK(not cib::service<partially_indexed_test_service>->handle(test_msg_t{
193+
"test_id_field"_field = 0x80, "test_opcode_field"_field = 2}));
194194
CHECK(not callback_success);
195195
}
196196

‎test/msg/indexed_handler.cpp‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ using msg_defn =
3434
message<"test_msg", opcode_field, sub_opcode_field, field_3, field_4>;
3535
using test_msg = owning<msg_defn>;
3636

37+
using callback_t = auto (*)(test_msg const &) -> bool;
38+
3739
template <auto N> using bitset = stdx::bitset<N, std::uint32_t>;
3840

3941
bitset<32> callbacks_called{};
@@ -45,7 +47,7 @@ TEST_CASE("create empty handler", "[indexed_handler]") {
4547
msg::indices{msg::index{
4648
opcode_field{}, lookup::make(CX_VALUE(
4749
lookup::input<std::uint32_t, bitset<32>>{}))}},
48-
std::array<void (*)(test_msg const &), 0>{}};
50+
std::array<callback_t, 0>{}};
4951
}
5052

5153
TEST_CASE("create handler with one index and callback", "[indexed_handler]") {
@@ -56,8 +58,10 @@ TEST_CASE("create handler with one index and callback", "[indexed_handler]") {
5658
lookup::make(CX_VALUE(lookup::input<std::uint32_t, bitset<32>, 1>{
5759
bitset<32>{}, std::array{lookup::entry{
5860
42u, bitset<32>{stdx::place_bits, 0}}}}))}},
59-
std::array<void (*)(test_msg const &), 1>{
60-
[](test_msg const &) { callbacks_called.set(0); }}};
61+
std::array<callback_t, 1>{[](test_msg const &) {
62+
callbacks_called.set(0);
63+
return true;
64+
}}};
6165

6266
callbacks_called.reset();
6367
CHECK(h.handle(test_msg{"opcode_field"_field = 42}));
@@ -97,16 +101,42 @@ TEST_CASE("create handler with multiple indices and callbacks",
97101
entry{2u, bitset<32>{stdx::place_bits, 2, 6, 8}},
98102
entry{3u, bitset<32>{stdx::place_bits, 3, 7, 8}},
99103
}}))}},
100-
std::array<void (*)(test_msg const &), 9>{
101-
[](test_msg const &) { callbacks_called.set(0); },
102-
[](test_msg const &) { callbacks_called.set(1); },
103-
[](test_msg const &) { callbacks_called.set(2); },
104-
[](test_msg const &) { callbacks_called.set(3); },
105-
[](test_msg const &) { callbacks_called.set(4); },
106-
[](test_msg const &) { callbacks_called.set(5); },
107-
[](test_msg const &) { callbacks_called.set(6); },
108-
[](test_msg const &) { callbacks_called.set(7); },
109-
[](test_msg const &) { callbacks_called.set(8); }}};
104+
std::array<callback_t, 9>{[](test_msg const &) {
105+
callbacks_called.set(0);
106+
return true;
107+
},
108+
[](test_msg const &) {
109+
callbacks_called.set(1);
110+
return true;
111+
},
112+
[](test_msg const &) {
113+
callbacks_called.set(2);
114+
return true;
115+
},
116+
[](test_msg const &) {
117+
callbacks_called.set(3);
118+
return true;
119+
},
120+
[](test_msg const &) {
121+
callbacks_called.set(4);
122+
return true;
123+
},
124+
[](test_msg const &) {
125+
callbacks_called.set(5);
126+
return true;
127+
},
128+
[](test_msg const &) {
129+
callbacks_called.set(6);
130+
return true;
131+
},
132+
[](test_msg const &) {
133+
callbacks_called.set(7);
134+
return true;
135+
},
136+
[](test_msg const &) {
137+
callbacks_called.set(8);
138+
return true;
139+
}}};
110140

111141
auto const check_msg = [&](std::uint32_t op, std::uint32_t sub_op,
112142
std::size_t callback_index) {
@@ -151,8 +181,11 @@ TEST_CASE("create handler with extra callback arg", "[indexed_handler]") {
151181
lookup::make(CX_VALUE(lookup::input<std::uint32_t, bitset<32>, 1>{
152182
bitset<32>{}, std::array{lookup::entry{
153183
42u, bitset<32>{stdx::place_bits, 0}}}}))}},
154-
std::array<void (*)(test_msg const &, std::size_t), 1>{
155-
[](test_msg const &, std::size_t i) { callbacks_called.set(i); }}};
184+
std::array<auto (*)(test_msg const &, std::size_t)->bool, 1>{
185+
[](test_msg const &, std::size_t i) {
186+
callbacks_called.set(i);
187+
return true;
188+
}}};
156189

157190
callbacks_called.reset();
158191
CHECK(h.handle(test_msg{"opcode_field"_field = 42}, std::size_t{1}));

0 commit comments

Comments
 (0)