Repository navigation
refactor: extract duplicated wire-unit arithmetic across mh/ipv6_route/hopopt/ipv6_opts - #509
Merged
Merged
Conversation
…popt/ipv6_opts (#495) Per-class helpers for read-side length arithmetic that had been copy-pasted across many call sites -- the same pattern that caused #487 and #398. - ipv6_route.py: add ipv6_route_header_length() beside the existing ipv6_route_data_length(), replacing the 4 `header.length * 8 + 8` sites in _read_data_type_*. Finishes the read-side half of #487/#489. - mh.py: add MH._mh_message_length(), replacing the 25 identical `(header.length + 1) * 8` sites in _read_msg_*. Mirrors the write side already unified at make() (`(len(data_val) + 6) // 8 - 1`). - mh.py, hopopt.py, ipv6_opts.py: add one per-class option-length helper each (_mh_option_length, _hopopt_option_length, _ipv6_opts_option_length), replacing the 92/17/17 = 126 `<schema>.length + 2` / `<schema>.len + 2` sites in _read_opt_*. This is the exact +2/-2 mismatch #398 fixed six times independently. Deliberately left alone: the hopopt.py/ipv6_opts.py shared base class (cross-cutting, its own issue) and the TCP/IP reassembly pair, which must NOT be merged since RFC 791 and RFC 9293 mandate opposite overlap resolution. Pure refactor, no behaviour change: full protocols test suite (466 passed, 1260 subtests) and test_option_roundtrip_unit.py (6 passed, 358 subtests) are identical before and after, and construct-then-parse byte comparisons for MH/IPv6-Route/HOPOPT/IPv6-Opts match exactly pre- and post-change.
JarryShaw
commented
Sep 19, 2026
This was referenced Sep 19, 2026
JarryShaw
added a commit
that referenced
this pull request
Sep 20, 2026
#528) Closes #512. Closes #517. `MH._mh_option_length` adds the 2 octets an RFC 6275 §6.2 mobility option spends on its 1-octet Option Type and 1-octet Option Length. The three CGA extension readers reused it, but a CGA extension's Extension Type and Extension Data Length are 2 octets each (RFC 4581 §2, which formally updates RFC 3972), so every parsed extension reported a length two octets short of what it consumed: the 8-octet `00120004deadbeef` came back as 6. - mh.py: add `_mh_extension_length`, returning `schema_length + 4`, and route `_read_ext_none`, `_read_ext_multiprefix` and `_read_ext_exp` through it. `_mh_option_length` is correct and is left alone -- the defect was in its callers, not in the helper. - mh.py: convert `_read_opt_pad`'s open-coded `clen + 2` to the helper #509 created to collect exactly that arithmetic, and add the `Note:` its own docstring already pointed at ("the surviving explanation of that fix ... below"), which had never travelled with the copied paragraph. - ipv6_opts.py: cite RFC 8200 §4.2, where the quoted `Opt Data Len` sentence actually appears, rather than §4.3 -- that is the Hop-by-Hop Options header, and this class implements Destination Options (§4.6). - tests: pin both header widths side by side, assert every CGA reader's reported length equals `len(schema.pack())`, and pin `_read_opt_pad`'s delegation by patching the helper with a sentinel. Corrects `test_mh_experimental_cga_extensions_round_trip`, which asserted the packed length as payload+4 and the parsed length as payload+2 four lines apart. tests/protocols/internet: 191 passed, 654 subtests, no regressions.
JarryShaw
added
ci
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…arning count Round 8: ran the merge-base/changed-files/cited-path intersection to completion (0c7f2b7..origin/main: 43 commits/124 files; 54 cited paths, 34 of them touched by that range) instead of trusting a tense grep. - :1540/:1542 -- "93 of the 95 sites"/"48 of the 49 record lengths" -> 94 of 95 / 49 of 49; zero old-expression sites remain on main. - :1544-1546, :1566 -- "LOCATOR_SET keeps the old expression ... is currently right" / "is unchanged in both respects" -> past tense; #679 fixed both LOCATOR_SET sites. - :1566 -- "HIP_COPIES stays at two" -> past tense; #679 fixed LOCATOR_SET's Length unit, #689 then dropped HIP_COPIES to one. - :1976-1977 -- "pcapng.txt ... wants a separate refresh" -> past tense; #685 removed it from the index instead of regenerating it. - :2327-2328 -- "55 warnings on main before this change, 56 after" (-b html) -> 53/54; the raw `grep -c WARNING:` double-counts two Scapy import lines as Sphinx warnings, confirmed live on this head with both -b dummy and -b html (real 36/raw 38 with const/reg.rst excluded, same +2 gap either way). - :834 -- stale ``protocol.py:1016`` -> ``:1413`` (the actual ``self._file.read()`` call inside ``_read_fileng``). - :1969 -- the #646 entry's coverage renumbering was wrong twice over (first ``1153 to 1265``, then ``1153 to 1443``, the latter being main's ``def`` line, which always executes and can never be the single miss). Corrected to ``1248 to 1360``, the ``warn(...)`` statement's line before/after #646's own diff. Regenerated CHANGELOG.md from the edited entries. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Follow-up: origin/main advanced through #726/#740/#741/#742 (to 0a3abff) and then #747/#748 (to 074c53e) while this sat at good-to-go; #726 moved three more claims anchored on files it touched. - :834 -- ``protocol.py:1413`` -> ``:1411``; #726 shifted the ``self._file.read()`` call in ``_read_fileng`` by -2 lines. - :2382-83 -- traceflow.py "Line 406" -> "Line 424"; #742 inserted 18 lines above the ``#: Type[Dumper]: Dumper class.`` comment. - :2099-2104, :2187-89 -- the "seven code-keyed parser registrars" and ``Option.register`` are no longer presence-only. #726, fixing #718, gave all seven -- and ``Option.register`` itself -- the same identity guard ``register_protocol`` already had; reworded both passages to say so, confirmed against the guards' own current docstrings. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 948ac49 came back NEEDS CHANGES: the round-12 edit fixed two sites of the harmonisation claim and left its twin, plus its own reasoning, asserting the opposite; and four numbers anchored on files the merges touched had drifted independently of #726. - :1877-1878, :1883-1884 (#675) -- "carries the guarded ``if code in cls.__xxx__: warn(...)``" / "every sibling warns on mere presence" -> past tense, noting #726 later gave all seven the identity guard this entry's own comparison assumes they lack. - :2100-2109 -- dropped the retained "yields two keys and never reaches one key twice" (false: ``Internet.register(TransType.TCP, TCP)`` warns once, incumbent.klass is TCP) and "leaves a different-class test undecidable" (contradicted by :2404-2406's own ``incumbent is not protocol`` definition); replaced with the actual false positive the guard has -- pre-seeded ``ModuleDescriptor`` incumbents never compare equal to the resolved class. - :2192 -- reflowed the ``Option.register`` paragraph (orphan lines fixed alongside). - :2387-2388 -- the ``Type[Dumper]`` quote now matches what is actually at line 424 (post-#709-fix), rather than the pre-fix bare form. - :945 -- ``README.md`` (103) -> (102). - :1136 -- "75 of the 117 modules" -> "77 ... after #647 below adds the same ending to three more" (drifted via #647, independent of the four merges). - :2119 -- dropped the irreproducible pylint "364 messages" figure; kept mypy's 112, which does reproduce. - :2119 -- "326 registry writes" -> 327 (``R1CounterParameter``'s second code, from #690). Also fixed six false claims in the PR body (separate from the .rst): hunk/line counts, six-commits -> 46, the 5-row table's implied total, "not trimmed", main's red/green state, and the now-unreachable cherry-pick target. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 1749cc0 came back NEEDS CHANGES: round 13 fixed five of the nine sites and introduced four new false claims doing it, including two inside the flagship rewrite -- swapping one inaccuracy for another is this document's recurring failure mode. - :1136-37 -- "75 ... 77 now, after #647 ... adds ... three more" was internally inconsistent (75+3=78, not 77). Traced #647's own diff (fc32d1b): it adds ``_missing_`` to three IntFlag classes across only two *new* files -- ``tcp/flags.py`` and ``ftp/command.py`` -- since the third, ``TransportProtocol``, shares ``reg/apptype.py`` with the already-counted ``AppType``. Module delta is +2, matching 75+2=77; reworded to say so. - :1879-88 -- dropped "the comparison below assumes they still lack" it, which was false about text 8 lines below in the same diff (already past-tensed). Also reflowed three orphan lines this introduced (`passes, whereas`, `it twice with nothing`, `none of the`). - :2107-19 -- "These tables also ship pre-seeded" over-generalised: verified live (``ProtocolBase.__proto__`` is 0 entries, ``Transport.__proto__ is ProtocolBase.__proto__`` -- True) that 2 of 7 have nothing pre-seeded. Scoped to the five that do (Link 7, Internet 16, Frame 3, PCAPNG 3, SCTP 2). Also fixed "the guard resolves only the incoming class", which contradicts the guard's own docstring ("the comparison itself resolves nothing") -- resolution is the earlier ``isinstance(protocol, ModuleDescriptor)`` step, three lines above the guard, not something the guard does. - :2129-30 -- dropped the invented "327th" ordinal (327 total stays; traced-write instrumentation via ``sys`` hooks found the seeding is literal dict construction, not ``.register()`` calls, so I could not reproduce an ordinal with confidence -- said "one of them" instead of guessing). - :7-8 -- "between #326 and #509" now says the programme continued past it (verified: 193 distinct #nnn refs, max #726, 103 above 509). - PR body -- "7 hunks, 1168+/11-" was the previous head's figure, not this one's; replaced with the actual command (``git diff --shortstat da697fa -- docs/source/changelog/1.5.0.rst``) and today's figure (9 hunks, 1152+/16-), since a hardcoded count here has now gone stale twice. Left alone per this round's scope: :1969/:1974 (before/after claim, not falsified by #726's later +1), mypy "112" (correct, re-ran with the project's own flags), ":2122" 13-to-14 (correct at its delta scope), and the other 121 cited paths (unaffected by main's one new commit, #745, confirmed test-only). Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests.
JarryShaw
added a commit
that referenced
this pull request
Sep 24, 2026
…arning count Round 8: ran the merge-base/changed-files/cited-path intersection to completion (0c7f2b7..origin/main: 43 commits/124 files; 54 cited paths, 34 of them touched by that range) instead of trusting a tense grep. - :1540/:1542 -- "93 of the 95 sites"/"48 of the 49 record lengths" -> 94 of 95 / 49 of 49; zero old-expression sites remain on main. - :1544-1546, :1566 -- "LOCATOR_SET keeps the old expression ... is currently right" / "is unchanged in both respects" -> past tense; #679 fixed both LOCATOR_SET sites. - :1566 -- "HIP_COPIES stays at two" -> past tense; #679 fixed LOCATOR_SET's Length unit, #689 then dropped HIP_COPIES to one. - :1976-1977 -- "pcapng.txt ... wants a separate refresh" -> past tense; #685 removed it from the index instead of regenerating it. - :2327-2328 -- "55 warnings on main before this change, 56 after" (-b html) -> 53/54; the raw `grep -c WARNING:` double-counts two Scapy import lines as Sphinx warnings, confirmed live on this head with both -b dummy and -b html (real 36/raw 38 with const/reg.rst excluded, same +2 gap either way). - :834 -- stale ``protocol.py:1016`` -> ``:1413`` (the actual ``self._file.read()`` call inside ``_read_fileng``). - :1969 -- the #646 entry's coverage renumbering was wrong twice over (first ``1153 to 1265``, then ``1153 to 1443``, the latter being main's ``def`` line, which always executes and can never be the single miss). Corrected to ``1248 to 1360``, the ``warn(...)`` statement's line before/after #646's own diff. Regenerated CHANGELOG.md from the edited entries. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Follow-up: origin/main advanced through #726/#740/#741/#742 (to 0a3abff) and then #747/#748 (to 074c53e) while this sat at good-to-go; #726 moved three more claims anchored on files it touched. - :834 -- ``protocol.py:1413`` -> ``:1411``; #726 shifted the ``self._file.read()`` call in ``_read_fileng`` by -2 lines. - :2382-83 -- traceflow.py "Line 406" -> "Line 424"; #742 inserted 18 lines above the ``#: Type[Dumper]: Dumper class.`` comment. - :2099-2104, :2187-89 -- the "seven code-keyed parser registrars" and ``Option.register`` are no longer presence-only. #726, fixing #718, gave all seven -- and ``Option.register`` itself -- the same identity guard ``register_protocol`` already had; reworded both passages to say so, confirmed against the guards' own current docstrings. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 948ac49 came back NEEDS CHANGES: the round-12 edit fixed two sites of the harmonisation claim and left its twin, plus its own reasoning, asserting the opposite; and four numbers anchored on files the merges touched had drifted independently of #726. - :1877-1878, :1883-1884 (#675) -- "carries the guarded ``if code in cls.__xxx__: warn(...)``" / "every sibling warns on mere presence" -> past tense, noting #726 later gave all seven the identity guard this entry's own comparison assumes they lack. - :2100-2109 -- dropped the retained "yields two keys and never reaches one key twice" (false: ``Internet.register(TransType.TCP, TCP)`` warns once, incumbent.klass is TCP) and "leaves a different-class test undecidable" (contradicted by :2404-2406's own ``incumbent is not protocol`` definition); replaced with the actual false positive the guard has -- pre-seeded ``ModuleDescriptor`` incumbents never compare equal to the resolved class. - :2192 -- reflowed the ``Option.register`` paragraph (orphan lines fixed alongside). - :2387-2388 -- the ``Type[Dumper]`` quote now matches what is actually at line 424 (post-#709-fix), rather than the pre-fix bare form. - :945 -- ``README.md`` (103) -> (102). - :1136 -- "75 of the 117 modules" -> "77 ... after #647 below adds the same ending to three more" (drifted via #647, independent of the four merges). - :2119 -- dropped the irreproducible pylint "364 messages" figure; kept mypy's 112, which does reproduce. - :2119 -- "326 registry writes" -> 327 (``R1CounterParameter``'s second code, from #690). Also fixed six false claims in the PR body (separate from the .rst): hunk/line counts, six-commits -> 46, the 5-row table's implied total, "not trimmed", main's red/green state, and the now-unreachable cherry-pick target. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at 1749cc0 came back NEEDS CHANGES: round 13 fixed five of the nine sites and introduced four new false claims doing it, including two inside the flagship rewrite -- swapping one inaccuracy for another is this document's recurring failure mode. - :1136-37 -- "75 ... 77 now, after #647 ... adds ... three more" was internally inconsistent (75+3=78, not 77). Traced #647's own diff (fc32d1b): it adds ``_missing_`` to three IntFlag classes across only two *new* files -- ``tcp/flags.py`` and ``ftp/command.py`` -- since the third, ``TransportProtocol``, shares ``reg/apptype.py`` with the already-counted ``AppType``. Module delta is +2, matching 75+2=77; reworded to say so. - :1879-88 -- dropped "the comparison below assumes they still lack" it, which was false about text 8 lines below in the same diff (already past-tensed). Also reflowed three orphan lines this introduced (`passes, whereas`, `it twice with nothing`, `none of the`). - :2107-19 -- "These tables also ship pre-seeded" over-generalised: verified live (``ProtocolBase.__proto__`` is 0 entries, ``Transport.__proto__ is ProtocolBase.__proto__`` -- True) that 2 of 7 have nothing pre-seeded. Scoped to the five that do (Link 7, Internet 16, Frame 3, PCAPNG 3, SCTP 2). Also fixed "the guard resolves only the incoming class", which contradicts the guard's own docstring ("the comparison itself resolves nothing") -- resolution is the earlier ``isinstance(protocol, ModuleDescriptor)`` step, three lines above the guard, not something the guard does. - :2129-30 -- dropped the invented "327th" ordinal (327 total stays; traced-write instrumentation via ``sys`` hooks found the seeding is literal dict construction, not ``.register()`` calls, so I could not reproduce an ordinal with confidence -- said "one of them" instead of guessing). - :7-8 -- "between #326 and #509" now says the programme continued past it (verified: 193 distinct #nnn refs, max #726, 103 above 509). - PR body -- "7 hunks, 1168+/11-" was the previous head's figure, not this one's; replaced with the actual command (``git diff --shortstat da697fa -- docs/source/changelog/1.5.0.rst``) and today's figure (9 hunks, 1152+/16-), since a hardcoded count here has now gone stale twice. Left alone per this round's scope: :1969/:1974 (before/after claim, not falsified by #726's later +1), mypy "112" (correct, re-ran with the project's own flags), ":2122" 13-to-14 (correct at its delta scope), and the other 121 cited paths (unaffected by main's one new commit, #745, confirmed test-only). Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests. Cross-review at b2ac58b came back NEEDS CHANGES: round 15 fixed four sites clean but swapped in two new inaccuracies, and left one round-fourteen defect (body :26) unfixed. - :7 -- "past #726" was wrong direction: #726 is the max ref in the document (193 distinct, min #251, max #726), not one exceeded -> "reaching #726". - :2110-19 -- "ProtocolBase and Transport share one dict, starting and staying empty until a subclass registers" was false three ways, verified live against origin/main (pcapkit.__file__ asserted): Transport.register() itself raises UnsupportedCall (abstract); TCP and UDP keep their own separate __proto__ (4 and 3 entries), not the shared one, so registering on them leaves the shared dict at 0; only a direct ProtocolBase.register() call fills it. Narrowing to "five of these seven" also hid that TCP/UDP are pre-seeded too, which is exactly where the false positive bites in the transport family -- restored that. - body :26 -- "26 entry commits" -> 27 (commits whose subject starts "docs(changelog): the 1.5.0 entry/entries for", verified by grep), 28 bullets added and 0 removed (verified via the .rst diff against da697fa; one commit, 6a956c4, adds two bullets for #648/#649). - body :41 -- dropped the hardcoded "9 hunks, 1152+/16-" figure entirely (it had already drifted to 1154+ by the time of this commit) and named the second command needed for the hunk count, since --shortstat cannot print one. On the ordinal question raised last round: dropping it was still right (the asserted "327th" was wrong), but "no ordinal is derivable" does not hold either -- the writes are at pcapkit/protocols/schema/schema.py, not the 8 dict-literal registrar sites my instrumentation covered, and they are traceable. Left the text as "one of them being R1CounterParameter's second code" (no ordinal asserted, no false derivability claim either) rather than reopen a site outside this round's scope. Regenerated CHANGELOG.md again. changelog_md.py --check: exit 0. pytest tests/project/test_changelog_md.py -q: 47 passed, 37 subtests.
2 of 5 tasks
JarryShaw
added a commit
that referenced
this pull request
Sep 29, 2026
…#900) - Turned the toctree-only changelog.rst index into a summary list: each of the 37 entries gets a one- or two-line summary drawn from its own page under changelog/, followed by :doc:`Full changelog <changelog/X.Y.Z>`. - The toctree itself moved to the bottom and gained :hidden: -- it is still the newest-first ordering util/changelog_md.py reads to pick the release CHANGELOG.md is generated from, just no longer the page's own rendering. - No entry under docs/source/changelog/ changes, and changelog.rst itself ships in neither the sdist nor CHANGELOG.md (MANIFEST.in prunes docs/ and re-adds only changelog/*.rst), so nothing shipped moves. - Cross-review fixes: dropped the 1.5.0 bullet's "~140 issues/PRs between #326 and #509" claim (wrong count, root cause is #657's own prose, raised there separately); dropped "silently" from 1.3.5, which the page itself contradicts; named the three workflows 1.4.1's unit-test CI gates instead of "every release pipeline"; reworded 0.15.5 for readability. - Added RepositoryStateTests.test_index_summary_bullets_track_each_pages_own_heading in tests/project/test_changelog_md.py: asserts the summary bullets' order matches read_toctree() and each bullet's (version, date) matches that page's own heading -- so 1.5.0 shipping with a real date and a stale "(unreleased)" bullet fails the suite instead of drifting silently. Verified failing against the pre-#909 toctree-only file (AssertionError naming all 37 missing bullets) before this commit added the bullets. Build/test: util/changelog_md.py --check exits 0; tests/project/ test_changelog_md.py passes (48 tests, 37 subtests); a Sphinx build into a fresh BUILDDIR reports Sphinx's own "42 warnings" summary line identically before and after, sorted WARNING lines byte-identical apart from timestamps/order on 4 unrelated dependency-check lines.
JarryShaw
added a commit
that referenced
this pull request
Sep 29, 2026
The paragraph claimed the defect programme ran "across some 140 issues and pull requests between #326 and #509" and reached "#805 by the entries below". Both are wrong, and re-measuring on this revision gives: * 254 distinct issue/PR references in the entries, not ~140. * 87 of them fall in [#326, #509]; 164 are above #509 and 3 below #326. * The highest is #883, not #805. The range framing was the root cause -- it encoded the window the programme opened on as though it were its extent, so every merge since made it more wrong. Replaced with a snapshot that says it is one, and regenerated CHANGELOG.md so the two stay in step (`util/changelog_md.py --check` exits 0).
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.
Closes #495 — the last open item from the post-wave-1 consistency sweep. Pure refactor: 168 duplicated arithmetic sites replaced by five helpers, with no behaviour change.
Why this is not tidying
Two shipped defects came from exactly this duplication. #487 computed
Hdr Ext Lenin four places and got four different wrong answers. #398 (29dfd351b) was one conceptual+2/-2unit mismatch that had to be diagnosed once and then patched independently in six files —hopopt.py130 lines,ipv6_opts.py131,mh.py130, plus all three schema files at 46/46/47.What changed
ipv6_route.pytotal header octetsipv6_route_header_length(), beside the existingipv6_route_data_length()mh.pymessage lengthMH._mh_message_length()mh.pyoption lengthMH._mh_option_length()hopopt.pyoption lengthHOPOPT._hopopt_option_length()ipv6_opts.pyoption lengthIPv6_Opts._ipv6_opts_option_length()The
mh.pymessage-length case is the clearest: the write side was already unified atmh.py:1438while the read side stayed duplicated 25 ways, and that value feeds_decode_next_layer(mh, schema.next, length - mh.length)— the same roleHdr Ext Lenplayed in #487.ipv6_route_header_length()is defined as4 + ipv6_route_data_length(hdr_ext_len), which keeps the two related-but-distinct quantities visibly related:8 + 8ktotal header octets versus4 + 8kdata octets. Confusing those two was #487.Deliberately NOT done
The
hopopt.py/ipv6_opts.pyshared base class. Those two files are one implementation twice (406 changed lines out of ~1950, overwhelmingly renaming), and collapsing them would subsume a large part of this issue — but it is a cross-cutting change that deserves its own issue and its own review, so the option-length helpers here are per-class rather than one shared helper across the two files. That boundary was drawn on purpose.Also untouched, and already retired with reasons in #495:
pcap.py/pcapng.pynot scaling fragment offsets (they consume already-scaled values, unlike the third-party adapters, so the asymmetry is correct);pypcap.py/pcap_ct.pyraisingUnsupportedCallthroughout; HIP's singlelength=schema.len * 8 + 8call site, which has nothing to share with.The trap this change had to avoid
reassembly/tcp.pyandreassembly/ip.pylook like near-duplicates and must not be merged: RFC 791 lines 1892-1894 mandate last-write-wins for IP, RFC 9293 §3.10 mandates first-write-wins for TCP, and #443 settled that deliberately. Neither file is in this diff — 0 reassembly files, verifiable fromgit diff --name-only origin/main...HEAD.Verification
Helper arithmetic checked independently of the tests — 240 comparisons over
k= 0..39 against the original expressions, 0 mismatches:Every old pattern is gone, and the counts match #495's measurements exactly:
Byte-identity against
main, which is the only proof that matters for a refactor. Construct-then-parse across IPv6-Route source-route with 0/1/2/3 addresses, six MH message types, and the HOPOPT/IPv6-Opts Router-Alert option, dumped to JSON from both trees:Critically, 0 error-ish lines in that output — so this is a comparison of real results, not of two identical crashes. (A driver that dies the same way on both trees reports "identical" while proving nothing; that happened earlier in this programme and is worth guarding against explicitly.)
Tests on the directly affected paths:
test_option_roundtrip_unit.py+test_mh_unit.py+test_ipv6_extension_unit.py→ 98 passed, 860 subtests passed.EXPECTED_FAILURESinspected by importing the module (it cannot be grepped —**unpacking): 59 entries, none needing an update.One deliberate omission
No dedicated unit tests for the five helpers. That follows the house precedent set by #487/#489:
IPv6_Route._make_hdr_ext_lenandipv6_route_data_lengthhave no dedicated tests either and are exercised through the existing round-trip suite. Adding a new convention here rather than following the existing one seemed the wrong call for a refactor, but it is a judgement worth disagreeing with — the arithmetic check above is a script, not a committed test.