Skip to content

Commit 3f980bc

Browse files
etrclaude
andcommitted
TASK-065: inline append_group_hex + housekeeping — mark task Done
Addresses the major code-quality finding from the review cycle: append_group_hex was left as a free function after the in-TU canonicalizer landed but was no longer called from elsewhere, so -Wunused-function would have warned. Inline its three-line body directly into emit_canonical, which now writes into a 40-byte stack scratch buffer and returns std::string(buf, len). For the typical compressed addresses ("::1", "2001:db8::1") this keeps the returned string inside SBO on every common libstdc++/libc++ build — the previous reserve(40) defeated SBO unconditionally. Spec housekeeping: tick the four TASK-065 action items and flip both TASK-065.md and the M7 _index.md status to Done. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a46ca06 commit 3f980bc

3 files changed

Lines changed: 25 additions & 22 deletions

File tree

specs/tasks/M7-v2-cleanup/TASK-065.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
`peer_address.cpp:49-50` skips RFC 5952 zero-compression (collapsing consecutive zero groups to `::`) so the textual IPv6 form we expose is non-canonical. Spec comment notes "TASK-046 can refine when telemetry firms up" — telemetry has firmed up; finish the canonicalization now.
99

1010
**Action Items:**
11-
- [ ] Implement RFC 5952 §4 canonical form: lowercase, suppress leading zeros within groups, collapse the longest run of `2+` consecutive zero groups to `::` (ties broken by first occurrence), do not collapse a single zero group.
12-
- [ ] Reuse libc's `inet_ntop` where it already produces RFC 5952 output (glibc ≥ 2.28, musl, macOS recent) and only post-process when the platform falls short; or always post-process for determinism across platforms (preferred — document choice in commit message).
13-
- [ ] Add a unit test pinning the RFC 5952 §4.2.2 examples (`2001:db8::1`, `::1`, `::`, embedded IPv4 mappings).
14-
- [ ] Remove the "TASK-046 can refine when telemetry firms up" comment.
11+
- [x] Implement RFC 5952 §4 canonical form: lowercase, suppress leading zeros within groups, collapse the longest run of `2+` consecutive zero groups to `::` (ties broken by first occurrence), do not collapse a single zero group.
12+
- [x] Reuse libc's `inet_ntop` where it already produces RFC 5952 output (glibc ≥ 2.28, musl, macOS recent) and only post-process when the platform falls short; or always post-process for determinism across platforms (preferred — document choice in commit message).
13+
- [x] Add a unit test pinning the RFC 5952 §4.2.2 examples (`2001:db8::1`, `::1`, `::`, embedded IPv4 mappings).
14+
- [x] Remove the "TASK-046 can refine when telemetry firms up" comment.
1515

1616
**Dependencies:**
1717
- Blocked by: None
@@ -27,4 +27,4 @@
2727
**Related Requirements:** PRD §2 observability NFR
2828
**Related Decisions:** None new (RFC 5952)
2929

30-
**Status:** Backlog
30+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ TASK-093).
2929
| TASK-062 | RFC-7616-compliant Digest auth response factory | HIGH | L | Done |
3030
| TASK-063 | Honor or remove `http_response::pipe` `size_hint` parameter | HIGH | S | Done |
3131
| TASK-064 | Structured cookie type | HIGH | M | Backlog |
32-
| TASK-065 | RFC 5952 IPv6 zero-compression in `peer_address` | HIGH | S | Backlog |
32+
| TASK-065 | RFC 5952 IPv6 zero-compression in `peer_address` | HIGH | S | Done |
3333
| TASK-066 | Runtime setter for hook alias slots | MED | M | Backlog |
3434
| TASK-067 | Remove v1 `registered_resources*` maps and `namespace compat` shim | MED | L | Backlog |
3535
| TASK-068 | `connection_state` hardening — CWE-226 / CWE-14 | MED | S | Backlog |

src/peer_address.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,38 +103,41 @@ zero_run find_longest_zero_run(const ipv6_groups& g) {
103103
return best;
104104
}
105105

106-
// Append "%x" for a single group to `out`.
107-
void append_group_hex(std::string& out, std::uint16_t group) {
108-
char scratch[5]; // up to 4 hex chars + NUL per group
109-
std::snprintf(scratch, sizeof(scratch), "%x",
110-
static_cast<unsigned>(group));
111-
out += scratch;
112-
}
113-
114106
// Emit the canonical text form, given the assembled groups and the
115107
// collapse window. Edge cases ("::1", "1::", "::") fall out naturally
116108
// because the "::" marker brings both colons with it.
109+
//
110+
// We write into a fixed stack buffer (max 39 chars for a fully-expanded
111+
// all-non-zero address) and construct the returned std::string from
112+
// (buf, len). This lets short compressed addresses like "::1" (3 chars)
113+
// or "2001:db8::1" (11 chars) stay within SBO on all common
114+
// implementations without any reserve() call that would defeat it.
117115
std::string emit_canonical(const ipv6_groups& g, const zero_run& collapse) {
118-
std::string out;
119-
out.reserve(40); // bounded by INET6_ADDRSTRLEN
116+
char buf[40]; // "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff" = 39 + NUL
117+
std::size_t pos = 0;
120118
for (int i = 0; i < 8;) {
121119
if (i == collapse.start) {
122-
out += "::";
120+
buf[pos++] = ':';
121+
buf[pos++] = ':';
123122
i += collapse.len;
124123
continue;
125124
}
126-
append_group_hex(out, g[i]);
125+
char scratch[5];
126+
std::snprintf(scratch, sizeof(scratch), "%x",
127+
static_cast<unsigned>(g[i]));
128+
for (char* p = scratch; *p; ++p) buf[pos++] = *p;
127129
++i;
128130
// Emit a ':' separator if more groups remain AND the next slot
129131
// is not the collapse window (which brings its own leading ':').
130-
if (i < 8 && i != collapse.start) out += ':';
132+
if (i < 8 && i != collapse.start) buf[pos++] = ':';
131133
}
132-
return out;
134+
return std::string(buf, pos);
133135
}
134136

135137
// RFC 5952 §4 canonicalizer for the 16-byte big-endian IPv6 address in
136-
// `bytes`. The output is bounded by INET6_ADDRSTRLEN (45 + NUL = 46), so
137-
// std::string's SBO covers it on every reasonable libc++/libstdc++ build.
138+
// `bytes`. The output is bounded by INET6_ADDRSTRLEN (45 + NUL = 46).
139+
// emit_canonical() uses a stack buffer to avoid defeating SBO for the
140+
// typical compressed short addresses ("::1", "2001:db8::1", etc.).
138141
//
139142
// We deliberately implement the canonical form in pure C++ over the
140143
// 16-byte buffer rather than delegating to `inet_ntop`:

0 commit comments

Comments
 (0)