Skip to content

fix(pcapng): name the Name Resolution Block in its guard messages (#1038) - #1047

Merged
JarryShaw merged 2 commits into
mainfrom
fix/1038-pcapng-nrb-messages
Oct 5, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
fix/1038-pcapng-nrb-messages

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort) — isort clean on pcapng.py; pylint/mypy not run (string-only source change)

  • make test passes, and a test case covers the change — tests added; test_pcapng_unit.py, test_option_roundtrip_unit.py and tests/project run locally

  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • chore — anything else

Description of your pull request and other information

Closes #1038. The six ns_dns* scope guards in pcapng.py, on both the read and the make side, check Name_Resolution_Block but named the systemd(1) Journal Export Block. They now say "must be in Name Resolution Block". register_record's RegistryWarning now says "name resolution record already registered". Neither message has a :manpage: role any more.

Spelling: ns_dnsIP4addr/ns_dnsIP6addr. That is the OptionType member name and value (const/pcapng/option_type.py:170,173), and the schema docstrings and read-side messages use it too. The make-side messages, the docstrings and the data-model docstrings now match.

Second commit, same defect class: I checked every bracketed option name against its own method. _make_option_if_rxspeed's two guards said [if_txspeed], and _read_option_epb_queue's length guard said [epb_packetid].

New tests cover every corrected message, and each one fails on main. The process.rst changelog pin is re-measured at 164 after rebasing onto #1046.

@JarryShaw
JarryShaw force-pushed the fix/1038-pcapng-nrb-messages branch from 19bf0ef to 13a93f2 Compare October 5, 2026 20:15
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 5, 2026
)

- The six ns_dnsname/ns_dnsIP4addr/ns_dnsIP6addr scope guards (read and
  make side) check Name_Resolution_Block but said the option must be in
  the systemd(1) Journal Export Block; they now name the Name Resolution
  Block, without the literal :manpage: role.
- register_record's RegistryWarning now says "name resolution record
  already registered" instead of "systemd(1) journal export record".
- The make-side ns_dnsIP4addr/ns_dnsIP6addr messages and docstrings use
  the OptionType member spelling instead of ns_dnsip4addr/ns_dnsip6addr.
- Tests cover every guard message and the warning; changelog entry and
  the process.rst entry-count pin (164) updated.
- _make_option_if_rxspeed's scope and duplicate guards said [if_txspeed];
  they now say [if_rxspeed].
- _read_option_epb_queue's length guard said [epb_packetid]; it now says
  [epb_queue].
- NS_DNSIP4AddrOption/NS_DNSIP6AddrOption data-model docstrings use the
  ns_dnsIP4addr/ns_dnsIP6addr spelling.
- Tests cover the three corrected messages; the #1038 changelog entry is
  extended (entry count unchanged at 164).
@JarryShaw
JarryShaw force-pushed the fix/1038-pcapng-nrb-messages branch from 3cf9659 to 4f55f49 Compare October 5, 2026 20:27
@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 5, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Cross-review verdict on 4f55f49e8: GOOD TO GO (ran on Sonnet; author Opus).

  • Every changed message now names what its guard checks. That covers the six NRB ns_dns* read/make guards, the register_record warning (__record__ holds only nrb_record_*), both _make_option_if_rxspeed guards, and _read_option_epb_queue's length guard.

  • The sweep is complete. An independent AST walk checked:

    • 160 bracketed messages against the OptionType/RecordType/SecretsType member each method maps to;
    • 136 "must be in X Block" texts against their Enum_BlockType guard.

    Both checks found 0 mismatches. As a control, the same walk on main reports exactly these 7 defects.

  • The spelling ns_dnsIP4addr/ns_dnsIP6addr matches option_type.py:170,173. The remaining lowercase ns_dnsip* strings are method-name suffixes and __option__ values, which are identifiers rather than messages.

  • Nothing pins the old text. The remaining "Journal Export Block" hits are the real systemd block.

  • The new tests fail on main and pass on head. On main: 12 failed, counting subtests. On head: test_pcapng_unit.py 97 passed, test_option_roundtrip_unit 6 passed.

  • The changelog is in step. changelog_md.py --check passes, and the pin reads 164, matching the measured 164.

Not breaking: only message text changes. Exception and warning types, raise conditions, signatures and pack paths are unchanged.

Nit: the last line of the 1.5.0 entry is not wrapped to the surrounding width.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 5, 2026
@JarryShaw
JarryShaw merged commit 8fcd114 into main Oct 5, 2026
11 checks passed
@JarryShaw
JarryShaw deleted the fix/1038-pcapng-nrb-messages branch October 5, 2026 21:07
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 5, 2026
@JarryShaw JarryShaw added this to the 1.5 milestone Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

pcapng: Name Resolution Block guards report the systemd Journal Export Block

1 participant