Skip to content

Commit 29d5148

Browse files
etrclaude
andcommitted
refactor: clarify ip_representation internals
The address store was uint16_t pieces[16] holding one octet (0-255) per element, which invited reading the elements as IPv6 hextets and needed a standing caveat comment to prevent it. weight() hand-rolled a variable-precision SWAR popcount over the mask. - uint16_t pieces[16] -> uint8_t octets[16]. Correct width for a byte, self-documenting name, and the "one octet per element despite the 16-bit type" caveat is gone. Octet-storage casts narrowed to uint8_t; the two v4-mapped-prefix helpers now take uint8_t octet values. - weight(): SWAR popcount -> std::popcount(mask) (<bit>, C++20). - documented `mask` (one bit per octet; cleared bit = '*' wildcard). No behavior change: http_utils_test asserts the parsed octets value-by- value (172 checks) and passes; full suite 109/109. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0d698e1 commit 29d5148

3 files changed

Lines changed: 208 additions & 209 deletions

File tree

src/detail/ip_representation.cpp

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,18 @@ uint16_t get_port(const struct sockaddr* sa) {
100100
}
101101

102102
ip_representation::ip_representation(const struct sockaddr* ip) {
103-
std::fill(pieces, pieces + 16, 0);
103+
std::fill(octets, octets + 16, 0);
104104
if (ip->sa_family == AF_INET) {
105105
ip_version = http_utils::IPV4;
106106
const in_addr* sin_addr_pt = &((reinterpret_cast<const struct sockaddr_in*>(ip))->sin_addr);
107107
for (int i = 0; i < 4; i++) {
108-
pieces[12 + i] = (reinterpret_cast<const u_char*>(sin_addr_pt))[i];
108+
octets[12 + i] = (reinterpret_cast<const u_char*>(sin_addr_pt))[i];
109109
}
110110
} else {
111111
ip_version = http_utils::IPV6;
112112
const in6_addr* sin_addr6_pt = &((reinterpret_cast<const struct sockaddr_in6*>(ip))->sin6_addr);
113113
for (int i = 0; i < 16; i++) {
114-
pieces[i] = (reinterpret_cast<const u_char*>(sin_addr6_pt))[i];
114+
octets[i] = (reinterpret_cast<const u_char*>(sin_addr6_pt))[i];
115115
}
116116
}
117117
mask = constants::DEFAULT_MASK_VALUE;
@@ -123,7 +123,7 @@ namespace {
123123
// be either 0x00 (pure ::-mapped) or 0xFF (v4-mapped). Lifted out of
124124
// parse_nested_ipv4 so the surrounding function stays below the CCN
125125
// bar.
126-
bool ipv4_mapped_prefix_invalid(uint16_t a, uint16_t b) {
126+
bool ipv4_mapped_prefix_invalid(uint8_t a, uint8_t b) {
127127
return (a != 0 && a != 255) || (b != 0 && b != 255);
128128
}
129129

@@ -132,15 +132,15 @@ bool ipv4_mapped_prefix_invalid(uint16_t a, uint16_t b) {
132132
// or 0xFF (v4-mapped). Throws invalid_argument on any other prefix.
133133
// Lifted out of parse_nested_ipv4 so the surrounding function stays
134134
// below the CCN bar.
135-
void validate_ipv4_mapped_prefix(const uint16_t* pieces) {
135+
void validate_ipv4_mapped_prefix(const uint8_t* octets) {
136136
for (unsigned int k = 0; k < 10; k++) {
137-
if (pieces[k] != 0) {
137+
if (octets[k] != 0) {
138138
throw std::invalid_argument(
139139
"IP is badly formatted. Nested IPV4 can be preceded only by 0 "
140140
"(and, optionally, two 255 octects)");
141141
}
142142
}
143-
if (ipv4_mapped_prefix_invalid(pieces[10], pieces[11])) {
143+
if (ipv4_mapped_prefix_invalid(octets[10], octets[11])) {
144144
throw std::invalid_argument(
145145
"IP is badly formatted. Nested IPV4 can be preceded only by 0 "
146146
"(and, optionally, two 255 octects)");
@@ -164,7 +164,7 @@ void ip_representation::parse_ipv4(const std::string& ip) {
164164
if (piece < 0 || piece > 255) {
165165
throw std::invalid_argument("IP is badly formatted. 255 is max value for ip part.");
166166
}
167-
pieces[12+i] = static_cast<uint16_t>(piece);
167+
octets[12+i] = static_cast<uint8_t>(piece);
168168
}
169169
}
170170

@@ -228,7 +228,7 @@ void ip_representation::parse_nested_ipv4(const std::vector<std::string>& parts,
228228
throw std::invalid_argument("IP is badly formatted. Nested IPV4 can have max 4 parts.");
229229
}
230230
// Bytes 0-9 must be zero; bytes 10-11 must be 0x00 or 0xFF.
231-
validate_ipv4_mapped_prefix(pieces);
231+
validate_ipv4_mapped_prefix(octets);
232232
for (unsigned int ii = 0; ii < subparts.size(); ii++) {
233233
if (subparts[ii] == "*") {
234234
clear_bit(mask, static_cast<unsigned int>(y + ii));
@@ -238,7 +238,7 @@ void ip_representation::parse_nested_ipv4(const std::vector<std::string>& parts,
238238
if (subpart < 0 || subpart > 255) {
239239
throw std::invalid_argument("IP is badly formatted. 255 is max value for ip part.");
240240
}
241-
pieces[y+ii] = static_cast<uint16_t>(subpart);
241+
octets[y+ii] = static_cast<uint8_t>(subpart);
242242
}
243243
}
244244

@@ -255,8 +255,8 @@ void ip_representation::apply_ipv6_part(std::vector<std::string>& parts, unsigne
255255
// Placeholder for one or more omitted segments. Zero-fill the
256256
// implied slots; the bump is `omitted` segments x 2 bytes each.
257257
for (unsigned int o = 0; o < omitted; o++) {
258-
pieces[y] = 0;
259-
pieces[y+1] = 0;
258+
octets[y] = 0;
259+
octets[y+1] = 0;
260260
y += 2;
261261
}
262262
return;
@@ -281,8 +281,8 @@ void ip_representation::apply_ipv6_part(std::vector<std::string>& parts, unsigne
281281
throw std::invalid_argument(
282282
"IP is badly formatted. IPV6 part contains a non-hex character.");
283283
}
284-
pieces[y] = static_cast<uint16_t>(hi);
285-
pieces[y+1] = static_cast<uint16_t>(lo);
284+
octets[y] = static_cast<uint8_t>(hi);
285+
octets[y+1] = static_cast<uint8_t>(lo);
286286
y += 2;
287287
return;
288288
}
@@ -308,7 +308,7 @@ void ip_representation::parse_ipv6(const std::string& ip) {
308308

309309
ip_representation::ip_representation(const std::string& ip) {
310310
mask = constants::DEFAULT_MASK_VALUE;
311-
std::fill(pieces, pieces + 16, 0);
311+
std::fill(octets, octets + 16, 0);
312312
if (ip.find(':') != std::string::npos) {
313313
parse_ipv6(ip);
314314
} else {
@@ -318,7 +318,7 @@ ip_representation::ip_representation(const std::string& ip) {
318318

319319
namespace {
320320

321-
// Add (16 - i) * piece[i] for both ip representations when both have
321+
// Add (16 - i) * octets[i] for both ip representations when both have
322322
// the i-th octet masked in. Pulled out of operator< so the surrounding
323323
// function stays below the CCN bar.
324324
void accumulate_octet_score(const ip_representation& a,
@@ -330,21 +330,21 @@ void accumulate_octet_score(const ip_representation& a,
330330
// is computed in the destination type. Without the cast the multiplication
331331
// happens in int and only the result is widened, which CodeQL flags as a
332332
// potential overflow.
333-
a_score += static_cast<int64_t>(16 - i) * a.pieces[i];
334-
b_score += static_cast<int64_t>(16 - i) * b.pieces[i];
333+
a_score += static_cast<int64_t>(16 - i) * a.octets[i];
334+
b_score += static_cast<int64_t>(16 - i) * b.octets[i];
335335
}
336336

337337
// True when both v4-mapped-prefix octets on a and b are 0x00 or 0xFF.
338338
// Mirrors the "::ffff:" / "::"-prefix invariant from parse_nested_ipv4.
339-
bool is_v4_mapped_prefix_octet_pair(uint16_t a, uint16_t b) {
339+
bool is_v4_mapped_prefix_octet_pair(uint8_t a, uint8_t b) {
340340
return (a == 0x00 || a == 0xFF) && (b == 0x00 || b == 0xFF);
341341
}
342342

343343
} // namespace
344344

345345
// Strict-weak ordering for std::set<ip_representation> (the allow and
346346
// deny lists). NOT lexicographic: each operand gets a position-weighted
347-
// score — the sum over octet index i of (16 - i) * pieces[i] — and the
347+
// score — the sum over octet index i of (16 - i) * octets[i] — and the
348348
// totals are compared. An octet contributes only when it is unmasked
349349
// on BOTH operands (see accumulate_octet_score), so wildcard octets on
350350
// either side drop out of the comparison entirely: a wildcard entry
@@ -375,8 +375,8 @@ bool ip_representation::operator <(const ip_representation& b) const {
375375
}
376376

377377
if (this_score == b_score
378-
&& is_v4_mapped_prefix_octet_pair(pieces[10], b.pieces[10])
379-
&& is_v4_mapped_prefix_octet_pair(pieces[11], b.pieces[11])) {
378+
&& is_v4_mapped_prefix_octet_pair(octets[10], b.octets[10])
379+
&& is_v4_mapped_prefix_octet_pair(octets[11], b.octets[11])) {
380380
return false;
381381
}
382382

src/httpserver/ip_representation.hpp

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <stdint.h>
2929

3030
#include <algorithm>
31+
#include <bit>
3132
#include <string>
3233
#include <vector>
3334

@@ -45,33 +46,31 @@ namespace http {
4546

4647
struct ip_representation {
4748
http_utils::IP_version_T ip_version;
48-
// One OCTET per element (values 0-255), despite the 16-bit element
49-
// type — do not read these as IPv6 hextets. A 4-hex-digit IPv6
50-
// group occupies TWO elements (high byte then low byte); an IPv4
51-
// or IPv4-mapped address occupies pieces[12..15]. See the parsers
52-
// in src/detail/ip_representation.cpp.
53-
uint16_t pieces[16];
49+
// The address as 16 octets, one per element. An IPv6 address fills all
50+
// 16 (each 4-hex-digit group is two octets: high byte then low byte);
51+
// an IPv4 (or IPv4-mapped) address fills octets[12..15] and leaves the
52+
// rest as the mapped prefix. See the parsers in
53+
// src/detail/ip_representation.cpp.
54+
uint8_t octets[16];
55+
// Bitmask over the 16 octets: bit i set means octet i is significant;
56+
// a cleared bit marks a '*' wildcard octet that matches anything.
5457
uint16_t mask;
5558

5659
explicit ip_representation(http_utils::IP_version_T ip_version) :
5760
ip_version(ip_version) {
5861
mask = constants::DEFAULT_MASK_VALUE;
59-
std::fill(pieces, pieces + 16, 0);
62+
std::fill(octets, octets + 16, 0);
6063
}
6164

6265
explicit ip_representation(const std::string& ip);
6366
explicit ip_representation(const struct sockaddr* ip);
6467

6568
bool operator<(const ip_representation& b) const;
6669

67-
// Returns the number of unmasked octets: the popcount of the 16-bit
68-
// mask (one bit per element of `pieces`). The magic constants below
69-
// are a variable-precision SWAR popcount.
70+
// Number of significant (non-wildcard) octets: the popcount of `mask`,
71+
// which carries one bit per octet.
7072
int weight() const {
71-
uint16_t x = mask;
72-
x = x - ((x >> 1) & 0x55555555);
73-
x = (x & 0x33333333) + ((x >> 2) & 0x33333333);
74-
return (((x + (x >> 4)) & 0x0F0F0F0F) * 0x01010101) >> 24;
73+
return std::popcount(mask);
7574
}
7675

7776
private:

0 commit comments

Comments
 (0)