Quellcode durchsuchen

MACAddress: AK::Array as member variable instead of C-array

Problem:
- C-style arrays do not automatically provide bounds checking and are
  less type safe overall.
- `__builtin_memcmp` is not a constant expression in the current gcc.

Solution:
- Change private m_data to be AK::Array.
- Eliminate constructor from C-style array.
- Change users of the C-style array constructor to use the default
  constructor.
- Change `operator==()` to be a hand-written comparison loop and let
  the optimizer figure out to use `memcmp`.
Lenny Maiorani vor 4 Jahren
Ursprung
Commit
bdf3baa8ac

+ 19 - 12
AK/MACAddress.h

@@ -26,22 +26,18 @@
 
 #pragma once
 
+#include <AK/Array.h>
 #include <AK/Assertions.h>
 #include <AK/String.h>
 #include <AK/Types.h>
 
 class [[gnu::packed]] MACAddress
 {
+    static constexpr size_t s_mac_address_length = 6u;
+
 public:
     constexpr MACAddress() = default;
 
-    constexpr MACAddress(const u8 data[6])
-    {
-        for (auto i = 0u; i < sizeof(m_data); ++i) {
-            m_data[i] = data[i];
-        }
-    }
-
     constexpr MACAddress(u8 a, u8 b, u8 c, u8 d, u8 e, u8 f)
     {
         m_data[0] = a;
@@ -54,15 +50,26 @@ public:
 
     constexpr ~MACAddress() = default;
 
-    constexpr u8 operator[](int i) const
+    constexpr const u8& operator[](unsigned i) const
     {
-        ASSERT(i >= 0 && i < 6);
+        ASSERT(i < s_mac_address_length);
+        return m_data[i];
+    }
+
+    constexpr u8& operator[](unsigned i)
+    {
+        ASSERT(i < s_mac_address_length);
         return m_data[i];
     }
 
     constexpr bool operator==(const MACAddress& other) const
     {
-        return !__builtin_memcmp(m_data, other.m_data, sizeof(m_data));
+        for (auto i = 0u; i < m_data.size(); ++i) {
+            if (m_data[i] != other.m_data[i]) {
+                return false;
+            }
+        }
+        return true;
     }
 
     String to_string() const
@@ -76,10 +83,10 @@ public:
     }
 
 private:
-    u8 m_data[6] {};
+    AK::Array<u8, s_mac_address_length> m_data {};
 };
 
-static_assert(sizeof(MACAddress) == 6);
+static_assert(sizeof(MACAddress) == 6u);
 
 namespace AK {
 

+ 16 - 9
AK/Tests/TestMACAddress.cpp

@@ -43,14 +43,6 @@ TEST_CASE(should_braces_construct)
     EXPECT(!sut.is_zero());
 }
 
-TEST_CASE(should_construct_from_c_array)
-{
-    constexpr u8 addr[6] = { 1, 2, 3, 4, 5, 6 };
-    constexpr MACAddress sut(addr);
-    static_assert(!sut.is_zero());
-    EXPECT(!sut.is_zero());
-}
-
 TEST_CASE(should_construct_from_6_octets)
 {
     constexpr MACAddress sut(1, 2, 3, 4, 5, 6);
@@ -58,7 +50,7 @@ TEST_CASE(should_construct_from_6_octets)
     EXPECT(!sut.is_zero());
 }
 
-TEST_CASE(should_provide_access_to_octet_by_index)
+TEST_CASE(should_provide_read_access_to_octet_by_index)
 {
     constexpr auto is_all_expected = [](auto& sut) {
         for (auto i = 0u; i < sizeof(MACAddress); ++i) {
@@ -78,6 +70,21 @@ TEST_CASE(should_provide_access_to_octet_by_index)
     }
 }
 
+TEST_CASE(should_provide_write_access_to_octet_by_index)
+{
+    constexpr auto sut = [] {
+        MACAddress m {};
+        for (auto i = 0u; i < sizeof(MACAddress); ++i) {
+            m[i] = i + 1;
+        }
+        return m;
+    }();
+
+    constexpr MACAddress expected(1, 2, 3, 4, 5, 6);
+
+    static_assert(expected == sut);
+}
+
 TEST_CASE(should_equality_compare)
 {
     constexpr MACAddress a(1, 2, 3, 4, 5, 6);

+ 2 - 1
Kernel/Net/E1000NetworkAdapter.cpp

@@ -24,6 +24,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <AK/MACAddress.h>
 #include <Kernel/IO.h>
 #include <Kernel/Net/E1000NetworkAdapter.h>
 #include <Kernel/Thread.h>
@@ -258,7 +259,7 @@ u32 E1000NetworkAdapter::read_eeprom(u8 address)
 void E1000NetworkAdapter::read_mac_address()
 {
     if (m_has_eeprom) {
-        u8 mac[6];
+        MACAddress mac {};
         u32 tmp = read_eeprom(0);
         mac[0] = tmp & 0xff;
         mac[1] = tmp >> 8;

+ 2 - 1
Kernel/Net/RTL8139NetworkAdapter.cpp

@@ -24,6 +24,7 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <AK/MACAddress.h>
 #include <Kernel/IO.h>
 #include <Kernel/Net/RTL8139NetworkAdapter.h>
 
@@ -284,7 +285,7 @@ void RTL8139NetworkAdapter::reset()
 
 void RTL8139NetworkAdapter::read_mac_address()
 {
-    u8 mac[6];
+    MACAddress mac {};
     for (int i = 0; i < 6; i++)
         mac[i] = in8(REG_MAC + i);
     set_mac_address(mac);