refactor!: framework-owned response emission + context regrouping (v2.0.0)#92
Open
w1ne wants to merge 44 commits into
Open
refactor!: framework-owned response emission + context regrouping (v2.0.0)#92w1ne wants to merge 44 commits into
w1ne wants to merge 44 commits into
Conversation
… NONE/after-action, regression-safe rollout)
…change persistence
…et via reset_pending
Convert uds_internal_handle_security_access and uds_internal_handle_authentication to v2 void signature with uds_result_t *out. Table entries for SID_SECURITY_ACCESS and SID_AUTHENTICATION now use handler_v2 column. The 0x29 handler retains ctx->suppress_pos_resp = false to ensure execute_handler always emits the positive response regardless of the request bit.
Convert read_memory_by_addr and write_memory_by_addr to v2 void handler signature. Move table entries for UDS_SID_READ_MEM_BY_ADDR and UDS_SID_WRITE_MEM_BY_ADDR to handler_v2 column (NULL in handler). All emission paths now use uds_ok/uds_nrc helpers; no uds_send_* calls.
Migrate uds_internal_handle_link_control (0x87) and uds_internal_handle_access_timing (0x83) to v2 void handler signature. Table entries in core_services[] updated to handler_v2 column (handler=NULL).
Migrate uds_internal_handle_io_control to v2 void handler signature. Point core_services[] UDS_SID_IO_CONTROL_BY_ID entry at handler_v2.
Convert uds_internal_handle_routine_control, request_download, transfer_data, request_transfer_exit, request_file_transfer, and request_upload to v2 void(ctx, data, len, uds_result_t*) contract. Point SIDs 0x31/0x34/0x35/0x36/0x37/0x38 at handler_v2 in core_services. Transfer-state logic (flash_sequence counter) unchanged.
Convert outer emission calls in uds_internal_handle_secured_data to the v2 contract (void handler with uds_result_t *out). All inner-dispatch and secure-capture machinery is byte-for-byte unchanged.
… is suppressed - Add UDS_RESULT_NONE kind and uds_none() helper to uds_config.h - Add case UDS_RESULT_NONE: break in execute_handler (no emit, no state change) - Fix uds_internal_handle_secured_data: suppressed-inner path uses uds_none() instead of uds_ok(out, 0u), preventing a spurious zero-length tp_send call - Add regression test: test_secured_inner_suppressed_emits_nothing asserts no outer frame is emitted when the inner 0x3E with suppressPosRsp bit is wrapped - Update design spec to reflect UDS_RESULT_NONE is retained for this one path
…P edge paths Adds test_core_coverage (logging filter, uds_client_request error/happy/mutex paths, uds_init validation/strict/NVM, uds_process S3/P2*/RCRRP/periodic, client response handling, busy-repeat, emit/send-NRC capture branches, secured-data errors) and test_tp_isotp_coverage (NULL guards, send-frame failure, N_Bs arm+expire, oversized/FF-failure sends, invalid escape FFs, wrong-SN CF, short FC, full TX flow STmin decode). Drives uds_core.c and uds_tp_isotp.c to near-full line coverage and closes the last uncovered function (uds_client_request).
Adds test_service_coverage, _coverage2 and _coverage3 covering reachable negative/error paths across the service handlers: - session (0x10) configured P2/P2* echo, safety session bit - security (0x27) seed-negative, handler-supplied NRC; auth (0x29) negative - IO control (0x2F) DID session/security denial, callback negative - link control (0x87) verify-specific baud, length errors, transition errors - access timing (0x83) set-given and readback - memory (0x23/0x3D) ALFID, too-long, parse-fail, callback-negative - flash (0x31/0x34/0x35/0x36/0x37/0x38) no-callback, ALFID, parse-fail, callback-negative, seq rollover, last-block replay, out-of-order, exit - data (0x22/0x24/0x2A/0x2C/0x2E) misconfigured DID, scaling/dynamic negative, WDBI session denial, periodic stop-all/update/table-full - maintenance (0x11/0x14/0x19) reset suppress, clear-negative, DTC list negative / response-too-long / per-record overflow / length errors across subfunctions 0x04/0x07/0x08/0x09/0x14/0x42/0x55/0x0B/0x0C/0x15, legacy fn_dtc_read negative - ROE (0x86) length/nesting/slot-full rejections
DTC store: register-update-existing, counter saturation at 0x7F and -128 floor, report on unknown DTC no-op, and extdata_cb (unknown/too-small/good). ResponseOnEvent (0x86): less-than/equal comparison operators, onDTCStatusChange match, reportActivatedEvents, clear, finite-window expiry, medium/slow timer rates, empty-capture emit, NULL trigger, serialize/deserialize argument and buffer guards.
Wires test_core_coverage, test_service_coverage{,2,3} and test_tp_isotp_coverage
into the ctest build.
Collapse the v1/v2 scaffold: make the out-param handler contract the sole public interface. Remove the legacy int-returning handler field and the temporary uds_handler_v2_t typedef from uds_service_entry_t. The public uds_service_handler_t is now void(*)(ctx, data, len, uds_result_t *out). execute_handler is unconditional: no legacy branch, no NULL guard. The emission switch (PENDING/NRC/NONE/POSITIVE) and post-emit reset block are unchanged. All 27 core_services[] initializers updated (NULL placeholder dropped, handler fn moved to position 5). All user_services handlers in tests and examples migrated to the out-param form. Version bumped to 2.0.0. CHANGELOG entry added with migration recipe.
…0 changelog heading
…versized inner response
…min gates only between CFs) uds_rx_fc() CTS branch left timer_st uninitialised, causing the STmin check to block the first CF until the raw timestamp exceeded STmin — violating ISO 15765-2 §6.5.5.5, which requires the first CF to be sent immediately. Add first_cf_after_fc flag: set on CTS, cleared after first CF is sent; subsequent CFs retain full STmin enforcement via timer_st.
…ized ROE response uds_internal_dispatch_captured() now resets secure_capture_overflow before dispatch (preventing stale-flag leakage from a prior 0x84 path) and returns UDS_ERR_BUFFER_TOO_SMALL on overflow instead of the ambiguous zero length. roe_emit_slot documents the explicit drop contract for the async ROE case where no live requester can receive an NRC. uds_send_response suppress-positive path also clears the overflow flag for consistency. fix(isotp): clear first_cf_after_fc on TX abort paths (defensive) Add iso->first_cf_after_fc = 0u to uds_rx_fc FC_OVA/default, uds_rx_sf half-duplex abort, and uds_rx_ff half-duplex abort so a stale flag cannot bypass STmin enforcement if a future refactor changes the reachability of ISOTP_TX_SENDING_CF without a fresh FC.CTS. Adds regression test test_roe_capture_overflow_drops_frame (64/64 pass).
Adds 12 stateful-flow regression tests covering: - Security gating, session-change re-lock, S3 timeout re-lock - Seed/session/key sequencing (NRC 0x24) - RCRRP recovery, NVM warm-start, auth deauth gate - suppressPosRsp for 0x85 and 0x2A - 0x84 inner overflow then ROE (cross-feature stale-flag clear) - comm_state persistence across S3 - Security lockout persistence across session change - Periodic subscription survival across S3 timeout
- Silence -Wunused-parameter/-Wunused-function in test_addressing_dispatch, test_async, test_concurrency, test_nvm_persistence, test_safety_gate via (void) casts - Give test_safety_check_passes a functional body and register it in test_safety_gate main() with setup_full/teardown - Remove dead setup/mock_service_handler/g_user_services in test_safety_gate (never registered, caused -Wunused-function)
…ment, remove duplicate test - Add test_seq_suppress_pos_rsp_19 (0x19 subfn 0x01 reportNumberOfDTCByStatusMask with bit7 set) to verify suppress flag does not leak into subsequent requests - Assert ctx.periodic_count == 1 after suppressed 0x2A subscribe in test_seq_suppress_pos_rsp_2A to prove the subscription was registered - Fix misleading comment in test_seq_nvm_warmstart_gated_works: security_mask is 2u (not 1), corrected to "level=1 satisfies security_mask=2u" - Remove test_pass from test_safety_gate.c; it was an exact behavioural duplicate of test_safety_check_passes (same g_safe_state, req, expected bytes)
Lock correct behaviour for SN wrap (19 CFs, 1..15→0..3), half-duplex FF-abort, N_Bs timeout after FC.WAIT, FF_DL exact-buffer-fit, FF_DL one-over-buffer (FC.OVFLW), STmin=0x7F, STmin=0xF1 microsecond band, and N_Cr timer refresh per CF (both pass and timeout variants).
…uite Locks the handle_request() check ordering in uds_core.c: - session (0x7F) before sub-length (0x13), sub validity (0x12), min-length (0x13), and security (0x33) - sub validity (0x12) before min-length (0x13) and security (0x33) - min-length (0x13) before security (0x33) - security (0x33) before safety hook (0x22) Also locks 0x36 mid-transfer mismatch (0x24), 0xFF->0x00 wrap, wrap mismatch; 0x84 inner-PENDING surfaced as encrypted NRC 0x78, inner-NRC encrypted and returned; functional 0x3E+suppress silent, functional session-gated NRC 0x7F suppressed. 15 new tests; 68/68 pass, 0 build warnings.
…yword false-match in comments
execute_handler now pre-inits the result descriptor to generalReject (0x10) and splits the POSITIVE case from a defensive default, so a handler that returns without describing a result is rejected rather than emitting stale tx_buffer contents. uds_ok/uds_nrc/uds_pending/uds_none fully initialise the descriptor so correctness no longer depends on caller pre-zeroing. Adds a fail-closed regression test and documents the 0x84 shared-buffer limitation and the host-simulation test scope in the changelog.
The stdlib scan used grep -rnw, so the word 'abort' in a comment ('clear on
TX abort') tripped the forbidden-call gate. Require a word boundary plus a
call paren so real abort()/malloc( are still caught while comment text and
identifiers like transfer_exit are not. Restores the precise 'abort' wording.
Per-PR CI only builds a subset of examples; the handler contract is public API and the examples are its migration guide, so they must not rot unbuilt. Adds a scheduled (and on-demand) job that builds and runs every host example against the Docker image, with success-marker assertions for the crypto pair.
Pure reformatting (no logic change): the session/security/server/client/scratch field renames shifted line lengths and comment alignment; clang-format-18 (the CI formatter) reflows them. Isolated from the substantive change for review.
…reset A 0x11 unwrapped from SecuredDataTransmission (0x84) ran its deferred reset inside the captured inner dispatch, rebooting before the outer secured response was emitted (and even when the outer returned an NRC). Gate the framework reset on !secure_capturing so it runs after the outer dispatch; cancel it on the 0x84 encode-fail / responseTooLong paths; and never reset from a captured ResponseOnEvent (0x86) dispatch. Adds nested-0x84 ordering regression tests (mutation-verified: both fail without the guard).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two staged architecture phases plus hardening, shipped together as v2.0.0 (breaking).
Phase 1 — framework-owned response emission. Response emission,
suppressPosRsp, and post-response ordering move out of every service handler into the dispatch framework.execute_handleris the single authority for how/when a response reaches the wire, removing the bug class behind #76 and #80 at its root.Phase 2 — context regrouping. The runtime
uds_ctx_t's ~40 flat fields are grouped intosession/security/server/client/scratchsub-structs. Per-dispatchscratchis reset at top-level entry, making per-request flag leaks (the #80 class) impossible by construction. Byte-identical — purely structural.Breaking changes
void handler(uds_ctx_t*, const uint8_t*, uint16_t, uds_result_t *out)withuds_ok/uds_nrc/uds_pending/uds_none. All core services + the 4 examples migrated.uds_send_response/uds_send_nrcremain public for app code.ctx->active_session→ctx->session.active,ctx->security_level→ctx->security.level, etc. Affects only code readinguds_ctx_tfields directly.Bug fixes shipped alongside
Tests & verification
Migration
See CHANGELOG 2.0.0. Handler:
return uds_send_response(ctx,n)→uds_ok(out,n);return uds_send_nrc(ctx,sid,nrc)→uds_nrc(out,nrc);return UDS_PENDING→uds_pending(out). Context: re-point any directuds_ctx_tfield reads at the new sub-structs.Known limitations (documented, not silent)
0x84 still shares
config->tx_bufferwith its copied-out inner dispatch under the single-threaded non-reentrant contract; the context-field layout may evolve in a future major; tests are host-simulation (mocked transport + virtual clock), not on-wire. Client/server split is Phase 3.