From 72d019f4a4acbf65ac6f6698f9efe74b46919106 Mon Sep 17 00:00:00 2001 From: Lenny Maiorani Date: Wed, 28 Oct 2020 10:50:53 -0600 Subject: [PATCH] IPv4Address: constexpr support Problem: - IPv4Address class cannot be used in a compile-time context. - A union is used by initializing one of the members and reading the non-active member. This is undefined behavior and not permitted in a `constexpr` context. Solution: - Eliminate undefined behavior by changing to a simple `u32` for storage instead of the union and performing mask/shift calculations for obtaining the individual octets. - Decorate functions with `constexpr` where possible. Currently string formatting and optionals are not `constexpr`-capable so functions using those are left out. - Modify tests to validate functionality in a `constexpr` context in addition to the run-time tests already being run. This ensures that functionality is the same in both contexts. --- AK/IPv4Address.h | 90 ++++++++++++++++++++---------------- AK/Tests/TestIPv4Address.cpp | 50 +++++++++++++------- 2 files changed, 82 insertions(+), 58 deletions(-) diff --git a/AK/IPv4Address.h b/AK/IPv4Address.h index 22da7c65784..5be98bd9550 100644 --- a/AK/IPv4Address.h +++ b/AK/IPv4Address.h @@ -33,69 +33,72 @@ #include #include -typedef u32 in_addr_t; - namespace AK { class [[gnu::packed]] IPv4Address { + enum class SubnetClass : int { + A = 0, + B, + C, + D + }; + public: - IPv4Address() { } - IPv4Address(const u8 data[4]) + using in_addr_t = u32; + + constexpr IPv4Address() = default; + + constexpr IPv4Address(u32 a, u32 b, u32 c, u32 d) { - m_data[0] = data[0]; - m_data[1] = data[1]; - m_data[2] = data[2]; - m_data[3] = data[3]; + m_data = (d << 24) | (c << 16) | (b << 8) | a; } - IPv4Address(u8 a, u8 b, u8 c, u8 d) + + constexpr IPv4Address(const u8 data[4]) { - m_data[0] = a; - m_data[1] = b; - m_data[2] = c; - m_data[3] = d; + m_data = (u32(data[3]) << 24) | (u32(data[2]) << 16) | (u32(data[1]) << 8) | u32(data[0]); } - IPv4Address(NetworkOrdered address) - : m_data_as_u32(address) + + constexpr IPv4Address(NetworkOrdered address) + : m_data(address) { } - u8 operator[](int i) const + constexpr u8 operator[](int i) const { ASSERT(i >= 0 && i < 4); - return m_data[i]; + return octet(SubnetClass(i)); } String to_string() const { - return String::formatted("{}.{}.{}.{}", m_data[0], m_data[1], m_data[2], m_data[3]); + return String::formatted("{}.{}.{}.{}", + octet(SubnetClass::A), + octet(SubnetClass::B), + octet(SubnetClass::C), + octet(SubnetClass::D)); } static Optional from_string(const StringView& string) { if (string.is_null()) return {}; - auto parts = string.split_view('.'); - u32 a; - u32 b; - u32 c; - u32 d; + const auto parts = string.split_view('.'); + + u32 a {}; + u32 b {}; + u32 c {}; + u32 d {}; if (parts.size() == 1) { - a = 0; - b = 0; - c = 0; d = parts[0].to_uint().value_or(256); } else if (parts.size() == 2) { a = parts[0].to_uint().value_or(256); - b = 0; - c = 0; d = parts[1].to_uint().value_or(256); } else if (parts.size() == 3) { a = parts[0].to_uint().value_or(256); b = parts[1].to_uint().value_or(256); - c = 0; d = parts[2].to_uint().value_or(256); } else if (parts.size() == 4) { a = parts[0].to_uint().value_or(256); @@ -108,32 +111,37 @@ public: if (a > 255 || b > 255 || c > 255 || d > 255) return {}; - return IPv4Address((u8)a, (u8)b, (u8)c, (u8)d); + return IPv4Address(a, b, c, d); } - in_addr_t to_in_addr_t() const { return m_data_as_u32; } - u32 to_u32() const { return m_data_as_u32; } + constexpr in_addr_t to_in_addr_t() const { return m_data; } + constexpr u32 to_u32() const { return m_data; } - bool operator==(const IPv4Address& other) const { return m_data_as_u32 == other.m_data_as_u32; } - bool operator!=(const IPv4Address& other) const { return m_data_as_u32 != other.m_data_as_u32; } + constexpr bool operator==(const IPv4Address& other) const = default; + constexpr bool operator!=(const IPv4Address& other) const = default; - bool is_zero() const + constexpr bool is_zero() const { - return m_data_as_u32 == 0; + return m_data == 0u; } private: - union { - u8 m_data[4]; - u32 m_data_as_u32 { 0 }; - }; + constexpr u32 octet(const SubnetClass subnet) const + { + NetworkOrdered address(m_data); + constexpr auto bits_per_byte = 8; + const auto bits_to_shift = bits_per_byte * int(subnet); + return (m_data >> bits_to_shift) & 0x0000'00FF; + } + + u32 m_data {}; }; static_assert(sizeof(IPv4Address) == 4); template<> struct Traits : public GenericTraits { - static unsigned hash(const IPv4Address& address) { return string_hash((const char*)&address, sizeof(address)); } + static constexpr unsigned hash(const IPv4Address& address) { return int_hash(address.to_u32()); } }; inline const LogStream& operator<<(const LogStream& stream, const IPv4Address& value) diff --git a/AK/Tests/TestIPv4Address.cpp b/AK/Tests/TestIPv4Address.cpp index a6d7fa4975b..bf35366f83a 100644 --- a/AK/Tests/TestIPv4Address.cpp +++ b/AK/Tests/TestIPv4Address.cpp @@ -31,31 +31,45 @@ TEST_CASE(should_default_contructor_with_0s) { - const IPv4Address addr {}; + constexpr IPv4Address addr {}; + + static_assert(addr.is_zero()); EXPECT(addr.is_zero()); } TEST_CASE(should_construct_from_c_array) { - const u8 a[4] = { 1, 2, 3, 4 }; - const IPv4Address addr(a); + constexpr auto addr = [] { + const u8 a[4] = { 1, 2, 3, 4 }; + return IPv4Address(a); + }(); + + static_assert(!addr.is_zero()); EXPECT(!addr.is_zero()); } TEST_CASE(should_construct_from_u32) { - const NetworkOrdered a = 0x11'22'33'44; - const IPv4Address addr(a); + constexpr auto addr = [] { + const NetworkOrdered a = 0x11'22'33'44; + return IPv4Address(a); + }(); + + static_assert(!addr.is_zero()); EXPECT(!addr.is_zero()); } TEST_CASE(should_get_octets_by_byte_offset) { - const u8 a[4] = { 1, 25, 39, 42 }; - const IPv4Address addr(a); + constexpr IPv4Address addr(1, 25, 39, 42); + + static_assert(1 == addr[0]); + static_assert(25 == addr[1]); + static_assert(39 == addr[2]); + static_assert(42 == addr[3]); EXPECT_EQ(1, addr[0]); EXPECT_EQ(25, addr[1]); @@ -65,8 +79,7 @@ TEST_CASE(should_get_octets_by_byte_offset) TEST_CASE(should_convert_to_string) { - const u8 a[4] = { 1, 25, 39, 42 }; - const IPv4Address addr(a); + constexpr IPv4Address addr(1, 25, 39, 42); EXPECT_EQ("1.25.39.42", addr.to_string()); } @@ -131,26 +144,29 @@ TEST_CASE(should_fill_a_b_d_octets_from_3_parts) TEST_CASE(should_convert_to_in_addr_t) { - const u8 a[4] = { 1, 2, 3, 4 }; - const IPv4Address addr(a); + constexpr IPv4Address addr(1, 2, 3, 4); + + static_assert(0x04'03'02'01u == addr.to_in_addr_t()); EXPECT_EQ(0x04'03'02'01u, addr.to_in_addr_t()); } TEST_CASE(should_convert_to_u32) { - const u8 a[4] = { 1, 2, 3, 4 }; - const IPv4Address addr(a); + constexpr IPv4Address addr(1, 2, 3, 4); + + static_assert(0x04'03'02'01u == addr.to_in_addr_t()); EXPECT_EQ(0x04'03'02'01u, addr.to_u32()); } TEST_CASE(should_compare) { - const u8 a[4] = { 1, 2, 3, 4 }; - const u8 b[4] = { 1, 2, 3, 5 }; - const IPv4Address addr_a(a); - const IPv4Address addr_b(b); + constexpr IPv4Address addr_a(1, 2, 3, 4); + constexpr IPv4Address addr_b(1, 2, 3, 5); + + static_assert(addr_a != addr_b); + static_assert(addr_a == addr_a); EXPECT(addr_a != addr_b); EXPECT(addr_a == addr_a);