Skip to content

Migrate IPv4 and HIP option dispatch from setattr/getattr to the registry pattern #429

Description

@JarryShaw

IPv4 and HIP dispatch their option/parameter handlers by building a method name and
getattr-ing it off the instance, while the other fourteen dispatch families go
through a class-level registry dict. Two idioms for one job.

The two forms

IPv4 / HIP — register_option installs the handler as a class attribute:

setattr(cls, f'_read_opt_{name}', meth[0])          # ipv4.py:458
setattr(cls, f'_read_param_{name}', meth[0])        # hip.py:590

and the read side derives the same name from the enum member:

meth_name = f'_read_opt_{name}'
meth = getattr(self, meth_name, self._read_opt_unassigned)

Everything else — a defaultdict keyed by code, holding a method-name string or a
(parser, constructor) pair:

name = self.__option__[kind]
meth = getattr(self, f'_read_mode_{name}', self._read_mode_donone)

Why migrating is worth it

  • Uniformity. 14 of 16 families use the registry; these two are the outliers, so
    anyone reading the codebase has to learn both.
  • Inspectability. len(TCP.__option__) is 22 and readable as data. IPv4's 15
    handlers can only be found by probing dir() for _read_opt_*, so "what is
    registered?" has no direct answer.
  • Registration is irreversible and namespace-global. setattr permanently installs
    a method on the class with no way to enumerate or undo it. The overwrite warning
    keys on hasattr(cls, f'_read_opt_{name}'), which is true for every shipped
    handler too, so it cannot distinguish a user registration from a built-in.

What it is not

Not a defect fix. I checked the hazard that would have made it one: across both enums
(IPv4 30 members, HIP 62) no member name collides with the
_read_opt_unassigned / _read_param_unassigned fallback, and none produces an
invalid identifier. The setattr form is not currently unsafe — it is just the less
legible of the two.

Worth noting the irony: the setattr form never had the lookup-miss leak that #425
fixed for the registry form, precisely because it has no dict to inject into.

Sequencing

After #427 and #428 merge. #428 reworks the ProtocolBase._lookup_registry helper a
migration would build on, and register_ipv4_option / register_hip_parameter are
public API — both signatures have to keep working while what happens underneath
changes, so the tests should pin the public behaviour before the internals move.

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

    enhancementIssues requesting a new capability (set by the feature request template)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions