Index NetHandler config by named enum

[brbzull0]

follow up for #13533.

Config was addressed as an array by advancing a pointer from its first member, and the update handler recovered the index by subtracting pointers to distinct members. Both are undefined behavior regardless of layout. Name the values instead so indexing is well defined, which also removes the layout assertions and the magic per-thread mask.

The index also arrives from an untyped event cookie, so give the enum a fixed underlying type: converting an out of range integer to an enumeration without one is undefined, which would defeat the check in operator[] before it could run. Scoping the enum keeps an int from silently becoming an index again.

The compiler checks the new switch for exhaustiveness but not for correctness, so add a test that every index reaches a distinct member. A case returning the wrong value would otherwise build cleanly and make a record update write to the wrong setting.

[Copilot]

Pull request overview

This PR removes undefined behavior in NetHandler configuration updates by replacing pointer-arithmetic “array indexing” with a named, scoped enum index that can be safely carried through the TS_EVENT_MGMT_UPDATE cookie path.

Changes:

• Introduces NetHandler::Config::Index (scoped, fixed underlying type) and rewrites Config::operator[] to switch on named indices rather than pointer arithmetic.
• Updates the config update callback and TS_EVENT_MGMT_UPDATE handling to pass/consume the enum index instead of reconstructing an index from unrelated member pointers.
• Adds a unit test to verify each Index maps to a distinct config member, and wires it into the test_net unit test target.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
include/iocore/net/NetHandler.h	Adds Config::Index and a switch-based operator[]; updates CONFIG_ITEM_COUNT to use Index::COUNT.
src/iocore/net/NetHandler.cc	Uses the enum index for record updates and event-cookie propagation instead of member pointer math.
src/iocore/net/UnixNet.cc	Replaces the magic mask with named enum-index bit selection for per-thread-dependent config.
src/iocore/net/unit_tests/test_NetHandler.cc	New Catch2 unit test asserting every config index maps to a distinct member.
src/iocore/net/CMakeLists.txt	Adds the new unit test source to the test_net target.

Suppressed comments (1)

include/iocore/net/NetHandler.h:33

• NetHandler.h uses std::bitset, std::numeric_limits, and uint32_t but doesn’t include the corresponding standard headers (<bitset>, <limits>, <cstdint>). Relying on transitive includes can break compilation if include order changes.

#include <atomic>

#include "tscore/ink_assert.h"

#include "iocore/eventsystem/Continuation.h"
#include "iocore/eventsystem/EThread.h"
#include "iocore/net/NetEvent.h"

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[brbzull0]

Took the 1ULL half . std::bitset's constructor takes unsigned long long anyway, so that's the right type to build the mask in.

I left out the static_assert. The case it guards is std::bitset silently dropping bits above its width, but that can't happen here anymore: the shift amounts are Config::Index enumerators, and every enumerator is by construction less than COUNT, which is exactly the width of the bitset (CONFIG_ITEM_COUNT). So the set bits are always in range.

That guard did earn its place in #13533, where the mask was a magic 0x3 with no connection to the struct it described and nothing tying the two together. Deriving the mask from named indices is what makes it redundant, so keeping it would preserve scaffolding for a problem the change removes. Happy to add it back if you'd rather have the belt and braces.