Skip to content

Commit 9985116

Browse files
committed
fix(api): Plumb safety + request IDs + array fields through singleton thunks
Closes the runtime gaps that surfaced once `-DNOTE_SINGLETON=1` started compiling against the polymorphic Notecard. Three independent singleton-mode bugs all routed through the same code path: (1) Safety dropped to NonIdempotent. The thunks called execute_void / execute_generic_with_body without a Safety parameter, so retry_loop saw NonIdempotent regardless of `RequestT::safety`. Idempotent / ReadOnly endpoints didn't retry on transient ResponseLost (CRC mismatch, read timeout). Adds Safety to the thunk function-pointer signature; generated `execute()` passes `safety` (the per-class constexpr). (2) Request IDs not injected. The static `execute()` template baked "id":N into its fields lambda; the singleton thunk path bypassed that lambda and the wire frame went out without an id. Adds `Notecard::next_request_id_or_zero()` (and the StaticNotecard mirror) and a `detail::IdWrapCtx` / `id_wrap_build` helper in protocol.hpp. The thunks wrap the caller's BuildFn with id_wrap so the id sits between "req" and the per-endpoint fields, matching the static template's wire format. (3) Version-guarded fields and StringArray fields silently dropped. The codegen previously filtered `version_guard` and `is_array` properties out of `field_descs_table_`; GenericResponseSink saw the bare scalar set, so e.g. CardAttn::off (gated) and CardAttn::files (array of strings) never landed in the parsed Response. Codegen now emits version-guarded entries with their own `#if` guards and emits a new `FieldType::StringArray` for string-element ResponseArray fields. `field_count` is computed from the post-preprocessor table size. GenericResponseSink picks up an `active_string_array_` latch in on_array_begin/on_array_end so on_string can append into the matching ResponseArray. The buffered fallback in `execute_generic_with_body` switches from `sax_parse` (emits only on_number for numerics, which GenericResponseSink only forwards to body handlers) to `sax_lex` (emits typed on_int / on_float / on_bool that the response field table actually dispatches). Also documents the Notecard::binary_ctrl_buf_[256] fixed buffer per Mat's earlier comment, and switches the err-stash from a fixed buffer to allocator-backed durable copy (heap-leaked, same model as StringPool::intern) so no new fixed-size buffer was added for these fixes. NOTE_MINIMAL gates: under NOTE_MINIMAL, NOTE_NO_RETRY and NOTE_NO_REQUEST_IDS are forced on, so safety and id wrapping are no-ops. The thunks short-circuit to a direct call. The StringArray dispatch in GenericResponseSink and the durable_ptr field in NcErrorCapture are also gated out. AVR delta after these fixes: avr-notecpp +78 B flash, RAM unchanged avr-notecpp-direct +64 B flash, RAM unchanged avr-notecpp-raw +0 B flash avr-notecpp-multistack -66 B flash (linker dedup of class-scope tables) The +78/+64 comes from version-guarded entries now landing in field_descs (one per gated property per endpoint) and Safety being threaded through `execute_generic_with_body`. Acceptable trade-off for closing three categories of correctness bugs that previously silently dropped fields. Singleton test failures: 73 → 0 (all 1858 host tests pass under `-DNOTE_SINGLETON=1`; default and NOTE_MINIMAL builds also pass).
1 parent d8006d4 commit 9985116

84 files changed

Lines changed: 1122 additions & 994 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/note/api.hpp

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,24 +156,39 @@ class Api {
156156
// `result.error().code` and ignore the string), so the +250 bytes
157157
// of flash for the durable-copy logic isn't worth it. The dangling
158158
// `view()` quirk persists there, gated to that minimal build only.
159-
static Result<void> void_thunk_(void* p, string_view req, BuildFn fn, void* ctx, detail::NcErrorCapture& err) {
159+
// The thunks plumb `Safety` (so retry_loop can decide whether
160+
// ResponseLost-style errors are retryable per-request) and inject
161+
// request IDs (`"id":N`) by wrapping the caller's fields BuildFn
162+
// with `id_wrap_build`. Both are needed for the singleton thunk
163+
// path to match the per-template `Notecard::execute()` semantics.
164+
//
165+
// Under NOTE_MINIMAL, NOTE_NO_RETRY and NOTE_NO_REQUEST_IDS are
166+
// forced on, so safety and request IDs have no observable effect —
167+
// skip the wrapping and `stash_nc_err` to keep AVR flash flat.
168+
// The function pointer signature stays uniform across builds so
169+
// the generated `execute()` doesn't need NOTE_MINIMAL branches.
170+
static Result<void> void_thunk_(void* p, string_view req, BuildFn fn, void* ctx, detail::NcErrorCapture& err, Safety safety) {
171+
auto* nc = static_cast<NcT*>(p);
160172
#if NOTE_MINIMAL
161-
return static_cast<NcT*>(p)->execute_void(req, fn, ctx, err);
173+
(void)safety;
174+
return nc->execute_void(req, fn, ctx, err);
162175
#else
163-
auto* nc = static_cast<NcT*>(p);
164-
auto rv = nc->execute_void(req, fn, ctx, err);
176+
detail::IdWrapCtx id_wrap{fn, ctx, nc->next_request_id_or_zero()};
177+
auto rv = nc->execute_void(req, &detail::id_wrap_build, &id_wrap, err, safety);
165178
err.set_durable(nc->stash_nc_err(err.view()));
166179
return rv;
167180
#endif
168181
}
169182
static Result<void> generic_thunk_(void* p, string_view req, BuildFn fn, void* ctx,
170183
void* rsp, const FieldDesc* f, uint8_t n, detail::NcErrorCapture& err, bool& ex,
171-
void* body_ptr, BodyHandlerFactory body_factory) {
184+
void* body_ptr, BodyHandlerFactory body_factory, Safety safety) {
185+
auto* nc = static_cast<NcT*>(p);
172186
#if NOTE_MINIMAL
173-
return static_cast<NcT*>(p)->execute_generic_with_body(req, fn, ctx, rsp, f, n, err, ex, body_ptr, body_factory);
187+
(void)safety;
188+
return nc->execute_generic_with_body(req, fn, ctx, rsp, f, n, err, ex, body_ptr, body_factory);
174189
#else
175-
auto* nc = static_cast<NcT*>(p);
176-
auto rv = nc->execute_generic_with_body(req, fn, ctx, rsp, f, n, err, ex, body_ptr, body_factory);
190+
detail::IdWrapCtx id_wrap{fn, ctx, nc->next_request_id_or_zero()};
191+
auto rv = nc->execute_generic_with_body(req, &detail::id_wrap_build, &id_wrap, rsp, f, n, err, ex, body_ptr, body_factory, safety);
177192
err.set_durable(nc->stash_nc_err(err.view()));
178193
return rv;
179194
#endif

src/note/api/card_attn.hpp

Lines changed: 56 additions & 58 deletions
Large diffs are not rendered by default.

src/note/api/card_aux.hpp

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -532,29 +532,30 @@ struct CardAux {
532532
std::unique_ptr<JsonReader> reader_;
533533
#endif
534534
};
535-
static constexpr uint8_t field_count = 3;
536-
static const ::note::FieldDesc* field_descs_ptr() {
537535
#pragma GCC diagnostic push
538536
#pragma GCC diagnostic ignored "-Winvalid-offsetof"
539-
static constexpr ::note::FieldDesc table[] NOTE_FLASH_ATTR = {
540-
{keys_::rsp_mode, static_cast<uint16_t>(offsetof(Response, mode)), ::note::FieldType::String},
541-
{keys_::rsp_seconds, static_cast<uint16_t>(offsetof(Response, seconds)), ::note::FieldType::Int},
542-
{keys_::rsp_time, static_cast<uint16_t>(offsetof(Response, time)), ::note::FieldType::Int},
543-
};
537+
static constexpr ::note::FieldDesc field_descs_table_[] NOTE_FLASH_ATTR = {
538+
{keys_::rsp_mode, static_cast<uint16_t>(offsetof(Response, mode)), ::note::FieldType::String},
539+
#if NOTE_API_VERSION >= NOTE_VERSION(3, 3, 1) || !defined(NOTE_API_STRICT)
540+
{keys_::rsp_power, static_cast<uint16_t>(offsetof(Response, power)), ::note::FieldType::Bool},
541+
#endif
542+
{keys_::rsp_seconds, static_cast<uint16_t>(offsetof(Response, seconds)), ::note::FieldType::Int},
543+
{keys_::rsp_time, static_cast<uint16_t>(offsetof(Response, time)), ::note::FieldType::Int},
544+
};
544545
#pragma GCC diagnostic pop
545-
return table;
546-
}
546+
static constexpr uint8_t field_count = sizeof(field_descs_table_) / sizeof(field_descs_table_[0]);
547+
static const ::note::FieldDesc* field_descs_ptr() { return field_descs_table_; }
547548

548549
#if NOTE_SINGLETON
549550
/// Singleton generic execute — shared thunk with body factory params.
550-
static inline Result<void>(*execute_generic_fn_)(void*, ::note::string_view, BuildFn, void*, void*, const ::note::FieldDesc*, uint8_t, ::note::detail::NcErrorCapture&, bool&, void*, ::note::BodyHandlerFactory);
551+
static inline Result<void>(*execute_generic_fn_)(void*, ::note::string_view, BuildFn, void*, void*, const ::note::FieldDesc*, uint8_t, ::note::detail::NcErrorCapture&, bool&, void*, ::note::BodyHandlerFactory, ::note::Safety);
551552
ApiResult<Response> execute() const {
552553
auto build_ = [&](JsonBuilder& b_) { this->build(b_); };
553554
BuildFn fn_ = [](JsonBuilder& b_, void* p_) { (*static_cast<decltype(build_)*>(p_))(b_); };
554555
Response rsp_{};
555556
::note::detail::NcErrorCapture nc_err_;
556557
bool exhausted_ = false;
557-
auto rv_ = execute_generic_fn_(nc_, notecard_request, fn_, &build_, &rsp_, field_descs_ptr(), field_count, nc_err_, exhausted_, nullptr, nullptr);
558+
auto rv_ = execute_generic_fn_(nc_, notecard_request, fn_, &build_, &rsp_, field_descs_ptr(), field_count, nc_err_, exhausted_, nullptr, nullptr, safety);
558559
if (!rv_) return ::note::Unexpected(rv_.error());
559560
if (!nc_err_.empty()) return ApiResult<Response>(::note::ErrorInfo{::note::Error::Notecard, ::note::Cause::Unspecified, nc_err_.view()});
560561
if (exhausted_) return ApiResult<Response>(::note::ErrorInfo{::note::Error::Overflow, ::note::Cause::Unspecified, NOTE_ERR("arena exhausted")});

src/note/api/card_aux_serial.hpp

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -355,27 +355,28 @@ struct CardAuxSerial {
355355
std::unique_ptr<JsonReader> reader_;
356356
#endif
357357
};
358-
static constexpr uint8_t field_count = 1;
359-
static const ::note::FieldDesc* field_descs_ptr() {
360358
#pragma GCC diagnostic push
361359
#pragma GCC diagnostic ignored "-Winvalid-offsetof"
362-
static constexpr ::note::FieldDesc table[] NOTE_FLASH_ATTR = {
363-
{keys_::rsp_mode, static_cast<uint16_t>(offsetof(Response, mode)), ::note::FieldType::String},
364-
};
360+
static constexpr ::note::FieldDesc field_descs_table_[] NOTE_FLASH_ATTR = {
361+
{keys_::rsp_mode, static_cast<uint16_t>(offsetof(Response, mode)), ::note::FieldType::String},
362+
#if NOTE_API_VERSION >= NOTE_VERSION(4, 1, 1) || !defined(NOTE_API_STRICT)
363+
{keys_::rsp_rate, static_cast<uint16_t>(offsetof(Response, rate)), ::note::FieldType::Int},
364+
#endif
365+
};
365366
#pragma GCC diagnostic pop
366-
return table;
367-
}
367+
static constexpr uint8_t field_count = sizeof(field_descs_table_) / sizeof(field_descs_table_[0]);
368+
static const ::note::FieldDesc* field_descs_ptr() { return field_descs_table_; }
368369

369370
#if NOTE_SINGLETON
370371
/// Singleton generic execute — shared thunk with body factory params.
371-
static inline Result<void>(*execute_generic_fn_)(void*, ::note::string_view, BuildFn, void*, void*, const ::note::FieldDesc*, uint8_t, ::note::detail::NcErrorCapture&, bool&, void*, ::note::BodyHandlerFactory);
372+
static inline Result<void>(*execute_generic_fn_)(void*, ::note::string_view, BuildFn, void*, void*, const ::note::FieldDesc*, uint8_t, ::note::detail::NcErrorCapture&, bool&, void*, ::note::BodyHandlerFactory, ::note::Safety);
372373
ApiResult<Response> execute() const {
373374
auto build_ = [&](JsonBuilder& b_) { this->build(b_); };
374375
BuildFn fn_ = [](JsonBuilder& b_, void* p_) { (*static_cast<decltype(build_)*>(p_))(b_); };
375376
Response rsp_{};
376377
::note::detail::NcErrorCapture nc_err_;
377378
bool exhausted_ = false;
378-
auto rv_ = execute_generic_fn_(nc_, notecard_request, fn_, &build_, &rsp_, field_descs_ptr(), field_count, nc_err_, exhausted_, nullptr, nullptr);
379+
auto rv_ = execute_generic_fn_(nc_, notecard_request, fn_, &build_, &rsp_, field_descs_ptr(), field_count, nc_err_, exhausted_, nullptr, nullptr, safety);
379380
if (!rv_) return ::note::Unexpected(rv_.error());
380381
if (!nc_err_.empty()) return ApiResult<Response>(::note::ErrorInfo{::note::Error::Notecard, ::note::Cause::Unspecified, nc_err_.view()});
381382
if (exhausted_) return ApiResult<Response>(::note::ErrorInfo{::note::Error::Overflow, ::note::Cause::Unspecified, NOTE_ERR("arena exhausted")});
@@ -677,12 +678,12 @@ struct CardAuxSerial {
677678

678679
#if NOTE_SINGLETON
679680
/// Singleton void execute — shared thunk, no per-type instantiation.
680-
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&);
681+
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&, ::note::Safety);
681682
ApiResult<void> execute() const {
682683
auto build_ = [&](JsonBuilder& b_) { this->build(b_); };
683684
BuildFn fn_ = [](JsonBuilder& b_, void* p_) { (*static_cast<decltype(build_)*>(p_))(b_); };
684685
::note::detail::NcErrorCapture nc_err_;
685-
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_);
686+
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_, safety);
686687
if (!rv_) return ::note::Unexpected(rv_.error());
687688
if (!nc_err_.empty()) return ApiResult<void>(::note::ErrorInfo{::note::Error::Notecard, ::note::Cause::Unspecified, nc_err_.view()});
688689
return ApiResult<void>{};
@@ -869,12 +870,12 @@ struct CardAuxSerial {
869870

870871
#if NOTE_SINGLETON
871872
/// Singleton void execute — shared thunk, no per-type instantiation.
872-
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&);
873+
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&, ::note::Safety);
873874
ApiResult<void> execute() const {
874875
auto build_ = [&](JsonBuilder& b_) { this->build(b_); };
875876
BuildFn fn_ = [](JsonBuilder& b_, void* p_) { (*static_cast<decltype(build_)*>(p_))(b_); };
876877
::note::detail::NcErrorCapture nc_err_;
877-
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_);
878+
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_, safety);
878879
if (!rv_) return ::note::Unexpected(rv_.error());
879880
if (!nc_err_.empty()) return ApiResult<void>(::note::ErrorInfo{::note::Error::Notecard, ::note::Cause::Unspecified, nc_err_.view()});
880881
return ApiResult<void>{};
@@ -1020,12 +1021,12 @@ struct CardAuxSerial {
10201021

10211022
#if NOTE_SINGLETON
10221023
/// Singleton void execute — shared thunk, no per-type instantiation.
1023-
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&);
1024+
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&, ::note::Safety);
10241025
ApiResult<void> execute() const {
10251026
auto build_ = [&](JsonBuilder& b_) { this->build(b_); };
10261027
BuildFn fn_ = [](JsonBuilder& b_, void* p_) { (*static_cast<decltype(build_)*>(p_))(b_); };
10271028
::note::detail::NcErrorCapture nc_err_;
1028-
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_);
1029+
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_, safety);
10291030
if (!rv_) return ::note::Unexpected(rv_.error());
10301031
if (!nc_err_.empty()) return ApiResult<void>(::note::ErrorInfo{::note::Error::Notecard, ::note::Cause::Unspecified, nc_err_.view()});
10311032
return ApiResult<void>{};
@@ -1146,12 +1147,12 @@ struct CardAuxSerial {
11461147

11471148
#if NOTE_SINGLETON
11481149
/// Singleton void execute — shared thunk, no per-type instantiation.
1149-
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&);
1150+
static inline Result<void>(*execute_void_fn_)(void*, ::note::string_view, BuildFn, void*, ::note::detail::NcErrorCapture&, ::note::Safety);
11501151
ApiResult<void> execute() const {
11511152
auto build_ = [&](JsonBuilder& b_) { this->build(b_); };
11521153
BuildFn fn_ = [](JsonBuilder& b_, void* p_) { (*static_cast<decltype(build_)*>(p_))(b_); };
11531154
::note::detail::NcErrorCapture nc_err_;
1154-
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_);
1155+
auto rv_ = execute_void_fn_(nc_, notecard_request, fn_, &build_, nc_err_, safety);
11551156
if (!rv_) return ::note::Unexpected(rv_.error());
11561157
if (!nc_err_.empty()) return ApiResult<void>(::note::ErrorInfo{::note::Error::Notecard, ::note::Cause::Unspecified, nc_err_.view()});
11571158
return ApiResult<void>{};

0 commit comments

Comments
 (0)