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
- Run build-test.yml once (add SUBMODULES_TOKEN) and keep it green.
- MISRA reports for RBC_GP, RBC_GA and Platform_OS_POSIX (needs the MISRA addon rule text or a licensed checker).
- A2: decide between an OSAdapter/RTE service for file and environment access and a documented deviation.
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.
- Q3: split the remaining high-complexity functions, starting with rte_voter_receive and rte_channel_checkpoint.
- 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.