RBC_Template 0.1.0 build 5fb9afb
Workspace master specification — safety-related railway software around an ERTMS Radio Block Centre (RBC), CENELEC EN 50128 / SIL 4
Loading...
Searching...
No Matches
Quality, architecture and software review, 2026-09-21

Scope: the C products (Platform_RTE, Platform_OS_POSIX, Platform_Protocol_SafeComm, RBC_GP, RBC_GA, RBC_SA), the workspace documents and the test environment. The review was done after the RCA layout and rte_ renames. It is a sampled review with tool support, not a line-by-line audit.

Status values: Fixed (changed and verified, see section 5), Partly (some of it done), Open (recorded in ../TODO.dox), Corrected (the first review statement was wrong or incomplete).

1. Method and limits

What How
Banned constructs grep for malloc/free/realloc/assert/errno/strcpy/sprintf in production sources
Static analysis cppcheck --enable=warning,performance,portability; clang-tidy readability-function-cognitive-complexity (threshold 25) and readability-function-size
Layering include and CMake reference scan (platform must not reach application)
Claim check every rule in docs/architecture/OVERVIEW.dox section 5 and docs/safety/SAFETY_AND_QUALITY_PLAN.dox section 3 compared with the code and build files
Build and test RBC_GP (with RTE, OS_POSIX, SA, GA) on macOS/clang and on Linux aarch64/gcc 12.2 (Debian, in Docker), Debug and Release, warnings as errors; Platform_RTE ctest

Not done: the cppcheck MISRA addon (no rule text available), coverage measurement, Robot suites, Docker image builds, an x86_64 build, and a run of the new CI workflow on GitHub. Complexity numbers are cognitive complexity from clang-tidy, not McCabe cyclomatic complexity; the framework rule (CLAUDE.md) is written in cyclomatic terms.

2. What is in good shape

  • platform/ never depends on application/; the only hit is a comment. safecomm::safety has no RTE include.
  • No malloc/calloc/realloc/free, assert or errno in Platform_RTE, SafeComm, RBC_GP or RBC_GA production code. Platform_OS_POSIX uses errno where the OS API requires it. The four strcpy calls in SafeComm are length-guarded.
  • cppcheck warnings: 0 in Platform_RTE, Platform_OS_POSIX, Platform_Protocol_SafeComm and RBC_GA before this pass.
  • Platform_RTE: 8 functions above cognitive complexity 25 in about 27k lines; 35 of 35 tests pass.
  • Documents: 0 broken links, product README plus CLAUDE.md per product, one architecture and one safety document.

3. Findings

3.1 Quality

ID Sev Finding Status
Q1 High The safety plan says every vital C product builds with -Wconversion -Wshadow -Werror. Only Platform_RTE and SafeComm did; RBC_GP and RBC_GA used -Wall -Wextra -Wpedantic without -Werror. Correction: Platform_OS_POSIX already gets the strict set when built through RTE (rte_set_warnings); only its stand-alone fallback was weaker. Fixed: GP, GA and the OS_POSIX fallback use -Wall -Wextra -Wpedantic -Wconversion -Wshadow -Werror (option RTE_WARNINGS_AS_ERRORS). Every warning this exposed on clang and gcc is fixed. The local test environment build still passes -DRTE_WARNINGS_AS_ERRORS=OFF on purpose.
Q2 High MISRA reports exist for Platform_RTE and SafeComm only. CI existed only for RTE and RBC_Test_Env. Correction: the POSIX OSAdapter's tests live in RBC_GP tests/ (not in its own repository), so "0 tests" was wrong; RBC_GA also has tests there (test_ab_ga_il_wire, test_ab_ga_route_fsm). Partly: .github/workflows/build-test.yml builds and tests all C products on Linux and macOS, Debug and Release, and gates cppcheck (written; needs the SUBMODULES_TOKEN secret and a first run). New tests: config loader (RBC_GP) and concurrent memory pool (POSIX OSAdapter). Open: MISRA reports for RBC_GP, RBC_GA, Platform_OS_POSIX.
Q3 Med Complexity above the framework rule. RTE (8 functions): rte_voter_receive 83, rte_channel_checkpoint 52, rte_dual_channel_send 45, rte_dual_channel_send_heartbeat 41, rte_redundancy_config_load 41, rte_osadapter_register_all 40. RBC_GP: 13 functions, worst gateway_c_config_load_file 360, resolve_channel 122, distance_from_lrbg_to_route_end 77. Partly: gateway_c_config_load_file split into 8 helper functions (worst remaining parse_channel_prop, 30). Open: the rest, listed in TODO.dox.
Q4 Med cppcheck on RBC_GP: possible NULL dereference in the role-C config parser and two uninitialised members in broker.hpp. Fixed. The NULL path was a real crash: a malformed value for a known key ("report_period_ms": abc) segfaulted the gateway at start-up. It now returns RTE_STATUS_INVALID_PARAM.
Q5 Low No automatic check that the Python wire mirror and the message catalogs follow the C header. Correction: sync_wire_catalog.py exists, but it printed 49 false "missing kind" errors (ETCS packet definitions have no envelope kind) and exited 0 in that case. Fixed: checks wire messages only and exits 1 on errors; included in the new CI job. Open: the simulators and RBC_Test_CommServer have no unit tests.
Q6 Low Platform_RTE/build and the DDS build still pointed at the pre-rename /cpp/SAPI tree, so tools reading compile_commands.json analysed nothing. Fixed: both removed, RTE regenerated (builds, 35 of 35 tests). Stale Platform_RTE/dist/Linux-* outputs remain (gitignored).

3.2 Architecture

ID Sev Finding Status
A1 High RBC_GP kept two diverged copies of common/ (AB/common, C/common). The C rbc_wire_types.h lacked message kinds 13-17 and 24-27; common_config.h differed by 163 lines. The wire layout is the contract with the Python mirror, so this made three copies. Fixed: one copy in AB/common; role C includes src/application/AB. Open: ab_ga_ertms_types.h and ab_gp_ertms_types.h are still byte-identical copies.
A2 High ARCHITECTURE.dox rule 3 (applications reach the OS only through RTE) is not true for the C library: vital A/B code uses <stdio.h>, <stdlib.h>, snprintf, getenv and file reads (about 17 includes in AB/). Fixed (as a formal deviation): every construct recorded by rule, file, line, justification and compensating measures in application/RBC_GP/docs/MISRA_COMPLIANCE_REPORT.dox. One candidate replacement (the site-startup-timestamp file, a natural fit for rte_nvm) was investigated and deliberately not made — its actual sharing semantics between A and B need a maintainer's confirmation first (see that report's Update 1). No RTE OSAdapter service exists yet for getenv or line-oriented text-config reads; building either is a real, separate platform feature.
A3 Med RBC_GP is 14.6k lines: vital A/B, non-vital role C, ERTMS procedures and site negotiation. Negotiation and channel code (ab_gp_channel*) is about 3.5k lines, and RTE also contains rte_dual_negotiator. That sits awkwardly with "the Platform decides, the application declares". The two negotiators were not compared in this review. Decided (2026-09-23): compared them directly. Not duplicates — rte_dual_negotiator is the generic Platform ONLINE/STANDBY primitive, ab_gp_channel_negotiate.c is real application-level orchestration on top of it (link setup, RBC-specific extra payload, promotion/demotion policy, reboot-lifecycle handling). Keeping both, as-is — see application/RBC_GP/CLAUDE.md's own record of this decision. One real finding fixed along the way: the application was reaching directly into the negotiator's private fields to resume identity across a reconnect; Platform_RTE gained a proper, additive API for that instead (rte_dual_negotiator_config_t::resume_own_state/resume_own_startup_timestamp_ms + diagnostic getters).
A4 Med GP, GA and SA reference each other at build time (documented). Conan cannot express it; it is worked around with required options. PI-API is not yet the only route from applications to RTE (documented in the architecture). Open, accepted.
A5 Med The local test environment and the Docker stack publish the same host ports (19100/19200/19300), so they cannot run together. setupLocalTestEnv.sh start reported "up" while the three simulators had died on the port, and the web runner then showed no train movement. Fixed: start refuses to run when the ports are busy and fails if a local process exits during start-up.

3.3 Software

ID Sev Finding Status
S1 Med The POSIX memory pool free list was not thread-safe (documented limitation) in an adapter with a SIL 4 target. Fixed: one mutex around create, acquire, release and stats. test_rte_posix_osadapter_memory_mt (8 threads) aborted or segfaulted on the old code in 3 of 3 runs and passed 30 of 30 on the new.
S2 Med GP ports and message_size come from both etc/config files and common_config.h (open ISSUE). ab-negotiate (1024) and the relays (96/256) still take message_size from JSON. Open (ISSUES.dox).
S3 Low CMake keeps dead fallbacks to pre-rename paths (platform/rte, os_adapters/posix, protocol_adapters/dds); about 158 lines in C sources still name safeAPI/SAPI, mostly comments. Open.
S4 Low rte_cast.c is 1208 lines and rte_string.c 732. Open.

3.4 Found while fixing

ID Finding Status
N1 A plain Linux build of RBC_GP (no toolchain file) failed to link librte_gp.so: non-PIC static site-data libraries. Fixed (CMAKE_POSITION_INDEPENDENT_CODE).
N2 On Linux test_ab_ga_il_wire and test_ab_ga_route_fsm ran against GP's dummy_ga (same file name as the real GA library) and failed with "undefined symbol". Fixed (BUILD_RPATH puts the real library first).
N3 nid_engine (32 bit) was cut to 8 bits when building the configuration Message 24; the bare-acknowledgement branch used 16 bits. Fixed.
N4 The TSR speed was narrowed from 32 to 16 bits without a check. Fixed (checked cast; a value that does not fit is refused).
N5 The relay tag TRAIN_POS_IN_ROUTE (18 characters) was silently cut to fit tag[16]. Fixed (TRAIN_POS_ROUTE; no consumer of the old text).
N6 GP tests were vacuous in Release: assert() compiled out, so calls inside asserts never ran, and one test crashed. Fixed (-UNDEBUG for the test directory).
N7 Four snprintf calls truncated silently; gcc Release with -Werror rejects them (the likely reason the local build turns -Werror off). Fixed (explicit %.*s).
N8 The role-C config scan hung on a stray value inside "channels": [...]. Fixed.

4. Recommended order for what is left

  1. Run build-test.yml once (add SUBMODULES_TOKEN) and keep it green.
  2. MISRA reports for RBC_GP, RBC_GA and Platform_OS_POSIX (needs the MISRA addon rule text or a licensed checker).
  3. A2: decide between an OSAdapter/RTE service for file and environment access and a documented deviation.
  4. A3: decide where site negotiation belongs, then remove one of the two negotiators or state the split. Done — decided to keep both (they are properly layered, not duplicates); see A3's own row above.
  5. Q3: split the remaining high-complexity functions, starting with rte_voter_receive and rte_channel_checkpoint.
  6. S2: one source for ports and message sizes.

5. Verification of the fixes

Check Result
RBC_GP + RTE + OS_POSIX + SA + GA, macOS clang, Debug and Release, -Werror -Wconversion -Wshadow builds; ctest 13 of 13
Same, Linux aarch64, gcc 12.2, Debug and Release builds with 0 warnings; ctest 13 of 13, three times; memory pool test 30 of 30
Platform_RTE, macOS, freshly regenerated build builds; ctest 35 of 35
cppcheck (warning, performance, portability) on RBC_GP 0 findings (was 5)
Memory pool test against the old code aborts or segfaults 3 of 3
Config loader test valid array and object forms, unknown properties, full channel table, 4 malformed inputs return an error
sync_wire_catalog.py exit 0 on the current tree
build-test.yml YAML parses; not run on GitHub

Not re-verified after the changes: Robot suites (local and Docker), Docker image builds (images still carry the old names), x86_64 gcc, Platform_RTE under -DCMAKE_BUILD_TYPE=Release.