Description
Follow-ups from the cross-review of #848, which introduced the "sink rows whose notes mention legacy to
the end" rule in pcapkit/vendor/reg/linktype.py. All three were judged non-blocking for that PR and
are recorded here rather than lost. None is a live defect today — a live fetch of
http://www.tcpdump.org/linktypes.html (205 rows) matches the rule on exactly one row,
209 / LINKTYPE_IPMB_LINUX / "Legacy names (do not use) for Linux I2C below.".
1. The rule is not value-aware, so it can invert in the other direction
sink = legacy if 'legacy' in cmmt.lower() else enum tests the notes column for the word alone. It does
not check whether the value is already claimed by an earlier row.
If a future current row's notes say something like "supersedes the legacy DLT_FOO" while its legacy twin's
notes do not, the rule sinks the current row and re-creates #844 in mirror image — silently, since
nothing asserts which of a duplicated pair is canonical beyond value 209.
A value-aware predicate would be immune: sink only when the row's value is already present in enum.
2. The range branch inherits the same sink, so one row can move sixteen members
The ValueError branch that expands a range (USER0–USER15, values 147–162) uses the same sink
variable. A single range row whose notes mention "legacy" would therefore sink all sixteen members at
once, not one.
3. The generator's comment oversells what the code does
The comment describes the sunk rows as
a legacy alias sharing its value with a later, current row
but the implemented predicate never checks value-sharing — it only looks for the word. Either make the code
match the comment (which is item 1) or make the comment match the code.
4. Cosmetic: the generated file is no longer numerically ordered
IPMB_LINUX = 209 now sits after DECT_NR_TAP = 304 at the end of the class body, because sinking appends.
Nothing tests for numeric ordering and the canonical iteration order is positionally unchanged — verified,
one positional difference in 219 slots — but a reader scanning the table will trip over it. Worth a comment
at the emission site at least.
Acceptance
- the predicate is value-aware, or the comment is corrected to describe what it actually tests
- a test pins which member is canonical for a duplicated value in both directions — i.e. one that fails if
a future regeneration sinks the current row instead of the legacy one
- the range-branch interaction is either prevented or covered by a test
Description
Follow-ups from the cross-review of #848, which introduced the "sink rows whose notes mention
legacytothe end" rule in
pcapkit/vendor/reg/linktype.py. All three were judged non-blocking for that PR andare recorded here rather than lost. None is a live defect today — a live fetch of
http://www.tcpdump.org/linktypes.html(205 rows) matches the rule on exactly one row,209 / LINKTYPE_IPMB_LINUX / "Legacy names (do not use) for Linux I2C below.".1. The rule is not value-aware, so it can invert in the other direction
sink = legacy if 'legacy' in cmmt.lower() else enumtests the notes column for the word alone. It doesnot check whether the value is already claimed by an earlier row.
If a future current row's notes say something like "supersedes the legacy
DLT_FOO" while its legacy twin'snotes do not, the rule sinks the current row and re-creates #844 in mirror image — silently, since
nothing asserts which of a duplicated pair is canonical beyond value 209.
A value-aware predicate would be immune: sink only when the row's value is already present in
enum.2. The range branch inherits the same
sink, so one row can move sixteen membersThe
ValueErrorbranch that expands a range (USER0–USER15, values 147–162) uses the samesinkvariable. A single range row whose notes mention "legacy" would therefore sink all sixteen members at
once, not one.
3. The generator's comment oversells what the code does
The comment describes the sunk rows as
but the implemented predicate never checks value-sharing — it only looks for the word. Either make the code
match the comment (which is item 1) or make the comment match the code.
4. Cosmetic: the generated file is no longer numerically ordered
IPMB_LINUX = 209now sits afterDECT_NR_TAP = 304at the end of the class body, because sinking appends.Nothing tests for numeric ordering and the canonical iteration order is positionally unchanged — verified,
one positional difference in 219 slots — but a reader scanning the table will trip over it. Worth a comment
at the emission site at least.
Acceptance
a future regeneration sinks the current row instead of the legacy one