Skip to content

#509 follow-ups: one call site left open-coded, a dangling cross-reference, and a wrong RFC section #517

Description

@JarryShaw

Three loose ends from #509 (PR #509, merged as 27bb315d5). None is a behaviour regression — the refactor is byte-identical, verified — but each undercuts a claim the refactor itself makes. The CGA length defect it also exposed is filed separately as #512.

1. mh.py:2853 still open-codes the arithmetic the helper was created to collect

pcapkit/protocols/internet/mh.py:2730-2732 states the intent:

collecting the read-side half of it into one helper is so a future fix to this arithmetic only has to happen once

But _read_opt_pad at :2853 still reads:

if code == Enum_Option.Pad1:
    size = 1
else:
    size = clen + 2

where the equivalent site in pcapkit/protocols/internet/hopopt.py:603 was converted:

_size = self._hopopt_option_length(schema.len)

ipv6_opts.py:612-615 was converted too. Only the mh.py branch was left behind, so the arithmetic still exists in two places in the file that introduced the helper.

2. A cross-reference that points at nothing

mh.py:2727-2730 refers the reader to "the surviving explanation of that fix … below". That Note: exists only in hopopt.py:579-588 and ipv6_opts.py:590-599; mh.py's _read_opt_pad has no Note: at all. The paragraph was copied and its "below" did not travel with it.

3. ipv6_opts.py:467 cites the wrong RFC section

Both helpers cite :rfc:8200#section-4.3``:

  • hopopt.py:456correct, §4.3 is the Hop-by-Hop Options Header
  • ipv6_opts.py:467wrong, IPv6-Opts is Destination Options, §4.6

The same file already uses #section-4.6 correctly for the same kind of claim at :1253. Introduced by #509's own new docstring, copied verbatim from hopopt.py. (§4.2 is arguably the right citation for both, since it defines the option format itself.)

Why these are worth a follow-up rather than a shrug

#509 was verified behaviour-neutral by byte-identity (md5 d797c44b0c1b on both trees), identical test counts (466 passed / 1260 subtests), and 240 independent arithmetic comparisons with 0 mismatches. Every one of those proofs tests that nothing changed — none of them tests whether the extraction was complete, which is exactly the gap items 1 and 2 sit in. Worth remembering for the next extraction refactor: a completeness check (grep the old pattern to zero, per file) is a different proof from a neutrality check.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions