Skip to content

Commit 40a27fe

Browse files
committed
Sync apps lookup wire header assertion
1 parent 02d2c07 commit 40a27fe

2 files changed

Lines changed: 127 additions & 3 deletions

File tree

.agents/sow/done/SOW-0008-20260602-sync-topology-containers-vendor-drift.md

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
Status: completed
66

7-
Sub-state: source sync, validation, artifact maintenance, and follow-up mapping are complete.
7+
Sub-state: reopened regression, repair, validation, artifact maintenance, and
8+
follow-up mapping are complete.
89

910
## Requirements
1011

@@ -463,3 +464,122 @@ Append regression entries here only after this SOW was completed or closed and
463464
later testing or use found broken behavior. Use a dated `## Regression -
464465
YYYY-MM-DD` heading at the end of the file. Never prepend regression content
465466
above the original SOW narrative.
467+
468+
## Regression - 2026-06-02
469+
470+
Status: completed
471+
472+
What broke:
473+
474+
- A new vendor diff appeared after the SOW was completed.
475+
- The drift is limited to
476+
`src/libnetdata/netipc/src/protocol/netipc_protocol.c`.
477+
- The C apps lookup static assertion still depended on the naturally padded
478+
`sizeof(nipc_apps_lookup_item_wire_t) == 64u` instead of asserting the fixed
479+
60-byte wire header boundary directly.
480+
481+
Evidence:
482+
483+
- `./diff-netdata-vendor.sh <topology-containers-tree> --unified` reported
484+
only this C file after expected exclusions and Go import normalization.
485+
- `ktsaou/netdata @ 8b652640fdeea7533bf81789d5119073c82b3b61`
486+
`src/libnetdata/netipc/src/protocol/netipc_protocol.c` asserts that
487+
`reserved1` ends at `NIPC_APPS_LOOKUP_ITEM_HDR_SIZE` and that the C struct
488+
covers at least that fixed header size.
489+
- `docs/codec-apps-lookup.md` records `NIPC_APPS_LOOKUP_ITEM_HDR_SIZE: 60`
490+
and explicitly says C must use explicit wire-size constants rather than
491+
`sizeof(struct)` as the apps item wire length.
492+
- `src/libnetdata/netipc/include/netipc/netipc_protocol.h` defines
493+
`NIPC_APPS_LOOKUP_ITEM_HDR_SIZE` as `60u`.
494+
495+
Why previous validation missed it:
496+
497+
- Previous validation was correct for the checked downstream revision at SOW
498+
completion time.
499+
- The checked topology containers tree later moved to
500+
`8b652640fdeea7533bf81789d5119073c82b3b61`, adding this narrower static
501+
assertion.
502+
503+
Repair plan:
504+
505+
1. Replace the padded `sizeof == 64u` assertion with explicit fixed-header
506+
boundary and minimum-coverage assertions.
507+
2. Run focused C protocol validation.
508+
3. Re-run the vendor diff script against the checked topology containers tree.
509+
4. Close this regression only after validation and artifact maintenance are
510+
recorded.
511+
512+
Validation plan:
513+
514+
- Build `test_protocol` if the existing build tree can compile it.
515+
- Run `ctest --test-dir build -R '^test_protocol$' --output-on-failure`.
516+
- Run `./diff-netdata-vendor.sh <topology-containers-tree>`.
517+
- Check SOW status/directory consistency and unrelated worktree changes.
518+
519+
Artifact updates needed:
520+
521+
- AGENTS.md: no update expected; this does not change project workflow.
522+
- Runtime project skills: no update expected; no runtime `project-*` skills
523+
exist.
524+
- Specs: no update expected; `docs/codec-apps-lookup.md` already records the
525+
exact wire-size rule this change enforces.
526+
- End-user/operator docs: no update expected; this is internal assertion
527+
hygiene with no public behavior change.
528+
- End-user/operator skills: no update expected; public integration guidance is
529+
unchanged.
530+
- SOW lifecycle: this SOW was reopened in `.agents/sow/current/`, validated,
531+
marked `Status: completed`, and moved back to `.agents/sow/done/`.
532+
533+
Repair performed:
534+
535+
- Replaced the padded `sizeof(nipc_apps_lookup_item_wire_t) == 64u` assertion
536+
with:
537+
- an assertion that `reserved1` ends at `NIPC_APPS_LOOKUP_ITEM_HDR_SIZE`
538+
byte 60;
539+
- an assertion that the C struct covers at least the fixed wire header size.
540+
541+
Validation results:
542+
543+
- `cmake --build build --target test_protocol`: passed.
544+
- `ctest --test-dir build -R '^test_protocol$' --output-on-failure`: failed
545+
before running tests because `~/.local/bin/ctest` is a Python wrapper
546+
missing the `cmake` module.
547+
- `/usr/bin/ctest --test-dir build -R '^test_protocol$' --output-on-failure`:
548+
passed; `test_protocol` passed.
549+
- `./diff-netdata-vendor.sh <topology-containers-tree>`: passed; no C, Rust,
550+
or Go source differences remain after expected exclusions and Go import-path
551+
normalization.
552+
- Same-failure search:
553+
`rg -n "sizeof\\(nipc_apps_lookup_item_wire_t\\)|fixed wire header must end|struct must cover the fixed wire header|NIPC_APPS_LOOKUP_ITEM_HDR_SIZE" src/libnetdata/netipc/src/protocol/netipc_protocol.c docs/codec-apps-lookup.md src/libnetdata/netipc/include/netipc/netipc_protocol.h`
554+
confirmed the old exact `sizeof` assertion is gone and the implementation,
555+
public header constant, and spec agree on the 60-byte wire header.
556+
557+
Artifact maintenance gate:
558+
559+
- AGENTS.md: no update needed; validation surfaced a local PATH wrapper issue,
560+
not a reusable project workflow change.
561+
- Runtime project skills: no update needed; no runtime `project-*` skills
562+
exist.
563+
- Specs: no update needed; `docs/codec-apps-lookup.md` already requires
564+
explicit wire-size constants and records `NIPC_APPS_LOOKUP_ITEM_HDR_SIZE` as
565+
60.
566+
- End-user/operator docs: no update needed; this changes an internal C compile
567+
assertion only.
568+
- End-user/operator skills: no update needed; public integration guidance is
569+
unchanged.
570+
- SOW lifecycle: reopened from `.agents/sow/done/` to `.agents/sow/current/`,
571+
marked `Status: completed` after validation, and moved back to
572+
`.agents/sow/done/` with the source fix.
573+
574+
Follow-up mapping:
575+
576+
- No follow-up work is required.
577+
- The `ctest` PATH wrapper failure is unrelated to this source change; the
578+
validation was completed with `/usr/bin/ctest`, so no source follow-up is
579+
required for this SOW.
580+
581+
Outcome:
582+
583+
- Completed. The upstream repository again matches the checked topology
584+
containers vendored NetIPC source after expected exclusions and Go import-path
585+
normalization.

src/libnetdata/netipc/src/protocol/netipc_protocol.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,12 @@ _Static_assert(offsetof(nipc_apps_lookup_item_wire_t, cgroup_name_offset) == 48,
227227
_Static_assert(offsetof(nipc_apps_lookup_item_wire_t, cgroup_name_length) == 52, "");
228228
_Static_assert(offsetof(nipc_apps_lookup_item_wire_t, label_count) == 56, "");
229229
_Static_assert(offsetof(nipc_apps_lookup_item_wire_t, reserved1) == 58, "");
230-
_Static_assert(sizeof(nipc_apps_lookup_item_wire_t) == 64u,
231-
"apps lookup C struct includes 4 bytes of trailing padding");
230+
_Static_assert(offsetof(nipc_apps_lookup_item_wire_t, reserved1) +
231+
sizeof(((nipc_apps_lookup_item_wire_t *)0)->reserved1) ==
232+
NIPC_APPS_LOOKUP_ITEM_HDR_SIZE,
233+
"apps lookup fixed wire header must end at byte 60");
234+
_Static_assert(sizeof(nipc_apps_lookup_item_wire_t) >= NIPC_APPS_LOOKUP_ITEM_HDR_SIZE,
235+
"apps lookup C struct must cover the fixed wire header");
232236

233237
/* Cgroups request (4 bytes) */
234238
_Static_assert(sizeof(nipc_cgroups_req_t) == 4,

0 commit comments

Comments
 (0)