Rewrite IPAddress without union

Using a union allowed undefined behavior for 8-bit integer INPUT and OUTPUT arguments through a 32-bit integer (and vice versa).
Write throught union::uint8_t[16] and read thoutght union::uint32_t[4]
is a undefined behavior.

C++ standard says (https://timsong-cpp.github.io/cppwp/n4659/class.union#1):
"At most one of the non-static data members of an object of union type
can be active at any time, that is, the value of at most one of the
non-static data members can be stored in a union at any time. [ Note:
One special guarantee is made in order to simplify the use of unions:
If a standard-layout union contains several standard-layout structs
that share a common initial sequence, and if a non-static data member
of an object of this standard-layout union type is active and is one
of the standard-layout structs, it is permitted to inspect the common
initial sequence of any of the standard-layout struct members; see
[class.mem].  — end note ]"

and (https://timsong-cpp.github.io/cppwp/n4659/class.mem#22):
"Two standard-layout unions are layout-compatible if they have the same number of non-static data members and corresponding non-static data members (in any order) have layout-compatible types."

you can't hope that compilers don't seem to do undefined behavior at the moment

Also created espressif/arduino-esp32#8829 PR dependent on changing the current API.

Author: safocl

api/IPAddress.cpp

@@ -19,53 +19,30 @@
 
 #include "IPAddress.h"
 #include "Print.h"
+#include <algorithm>
+#include <cstdint>
 
 using namespace arduino;
 
-IPAddress::IPAddress() : IPAddress(IPv4) {}
+IPAddress::IPAddress() = default;
 
-IPAddress::IPAddress(IPType ip_type)
-{
-    _type = ip_type;
-    memset(_address.bytes, 0, sizeof(_address.bytes));
-}
+IPAddress::IPAddress(IPType ip_type) : _type(ip_type) {}
 
 IPAddress::IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet)
 {
-    _type = IPv4;
-    memset(_address.bytes, 0, sizeof(_address.bytes));
-    _address.bytes[IPADDRESS_V4_BYTES_INDEX] = first_octet;
-    _address.bytes[IPADDRESS_V4_BYTES_INDEX + 1] = second_octet;
-    _address.bytes[IPADDRESS_V4_BYTES_INDEX + 2] = third_octet;
-    _address.bytes[IPADDRESS_V4_BYTES_INDEX + 3] = fourth_octet;
+    _address[IPADDRESS_V4_BYTES_INDEX] = first_octet;
+    _address[IPADDRESS_V4_BYTES_INDEX + 1] = second_octet;
+    _address[IPADDRESS_V4_BYTES_INDEX + 2] = third_octet;
+    _address[IPADDRESS_V4_BYTES_INDEX + 3] = fourth_octet;
 }
 
-IPAddress::IPAddress(uint8_t o1, uint8_t o2, uint8_t o3, uint8_t o4, uint8_t o5, uint8_t o6, uint8_t o7, uint8_t o8, uint8_t o9, uint8_t o10, uint8_t o11, uint8_t o12, uint8_t o13, uint8_t o14, uint8_t o15, uint8_t o16) {
-    _type = IPv6;
-    _address.bytes[0] = o1;
-    _address.bytes[1] = o2;
-    _address.bytes[2] = o3;
-    _address.bytes[3] = o4;
-    _address.bytes[4] = o5;
-    _address.bytes[5] = o6;
-    _address.bytes[6] = o7;
-    _address.bytes[7] = o8;
-    _address.bytes[8] = o9;
-    _address.bytes[9] = o10;
-    _address.bytes[10] = o11;
-    _address.bytes[11] = o12;
-    _address.bytes[12] = o13;
-    _address.bytes[13] = o14;
-    _address.bytes[14] = o15;
-    _address.bytes[15] = o16;
-}
+IPAddress::IPAddress(uint8_t o1, uint8_t o2, uint8_t o3, uint8_t o4, uint8_t o5, uint8_t o6, uint8_t o7, uint8_t o8, uint8_t o9, uint8_t o10, uint8_t o11, uint8_t o12, uint8_t o13, uint8_t o14, uint8_t o15, uint8_t o16) : _address{o1, o2, o3, o4, o5, o6, o7, o8, o9, o10, o11, o12, o13, o14, o15, o16}, _type(IPv6) {}
 
-IPAddress::IPAddress(uint32_t address)
+// IPv4 only
+IPAddress::IPAddress(uint32_t address) 
 {
-    // IPv4 only
-    _type = IPv4;
-    memset(_address.bytes, 0, sizeof(_address.bytes));
-    _address.dword[IPADDRESS_V4_DWORD_INDEX] = address;
+    uint32_t& addressRef = reinterpret_cast<uint32_t&>(_address[IPADDRESS_V4_BYTES_INDEX]);
+    addressRef = address;
 
     // NOTE on conversion/comparison and uint32_t:
     // These conversions are host platform dependent.
@@ -78,14 +55,12 @@ IPAddress::IPAddress(uint32_t address)
 
 IPAddress::IPAddress(const uint8_t *address) : IPAddress(IPv4, address) {}
 
-IPAddress::IPAddress(IPType ip_type, const uint8_t *address)
+IPAddress::IPAddress(IPType ip_type, const uint8_t *address) : _type(ip_type)
 {
-    _type = ip_type;
     if (ip_type == IPv4) {
-        memset(_address.bytes, 0, sizeof(_address.bytes));
-        memcpy(&_address.bytes[IPADDRESS_V4_BYTES_INDEX], address, sizeof(uint32_t));
+        std::copy(address, address + 4, &_address[IPADDRESS_V4_BYTES_INDEX]);
     } else {
-        memcpy(_address.bytes, address, sizeof(_address.bytes));
+        std::copy(address, address + _address.size(), _address.begin());
     }
 }
 
@@ -97,18 +72,18 @@ IPAddress::IPAddress(const char *address)
 String IPAddress::toString4() const
 {
     char szRet[16];
-    snprintf(szRet, sizeof(szRet), "%u.%u.%u.%u", _address.bytes[IPADDRESS_V4_BYTES_INDEX], _address.bytes[IPADDRESS_V4_BYTES_INDEX + 1], _address.bytes[IPADDRESS_V4_BYTES_INDEX + 2], _address.bytes[IPADDRESS_V4_BYTES_INDEX + 3]);
+    snprintf(szRet, sizeof(szRet), "%u.%u.%u.%u", _address[IPADDRESS_V4_BYTES_INDEX], _address[IPADDRESS_V4_BYTES_INDEX + 1], _address[IPADDRESS_V4_BYTES_INDEX + 2], _address[IPADDRESS_V4_BYTES_INDEX + 3]);
     return String(szRet);
 }
 
 String IPAddress::toString6() const
 {
     char szRet[40];
     snprintf(szRet, sizeof(szRet), "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x",
-            _address.bytes[0], _address.bytes[1], _address.bytes[2], _address.bytes[3],
-            _address.bytes[4], _address.bytes[5], _address.bytes[6], _address.bytes[7],
-            _address.bytes[8], _address.bytes[9], _address.bytes[10], _address.bytes[11],
-            _address.bytes[12], _address.bytes[13], _address.bytes[14], _address.bytes[15]);
+            _address[0], _address[1], _address[2], _address[3],
+            _address[4], _address[5], _address[6], _address[7],
+            _address[8], _address[9], _address[10], _address[11],
+            _address[12], _address[13], _address[14], _address[15]);
     return String(szRet);
 }
 
@@ -135,7 +110,7 @@ bool IPAddress::fromString4(const char *address)
     int16_t acc = -1; // Accumulator
     uint8_t dots = 0;
 
-    memset(_address.bytes, 0, sizeof(_address.bytes));
+    _address.fill(0);
     while (*address)
     {
         char c = *address++;
@@ -157,7 +132,7 @@ bool IPAddress::fromString4(const char *address)
                 /* No value between dots, e.g. '1..' */
                 return false;
             }
-            _address.bytes[IPADDRESS_V4_BYTES_INDEX + dots++] = acc;
+            _address[IPADDRESS_V4_BYTES_INDEX + dots++] = acc;
             acc = -1;
         }
         else
@@ -175,7 +150,7 @@ bool IPAddress::fromString4(const char *address)
         /* No value between dots, e.g. '1..' */
         return false;
     }
-    _address.bytes[IPADDRESS_V4_BYTES_INDEX + 3] = acc;
+    _address[IPADDRESS_V4_BYTES_INDEX + 3] = acc;
     _type = IPv4;
     return true;
 }
@@ -215,8 +190,8 @@ bool IPAddress::fromString6(const char *address) {
             if (colons == 7)
                 // too many separators
                 return false;
-            _address.bytes[colons * 2] = acc >> 8;
-            _address.bytes[colons * 2 + 1] = acc & 0xff;
+            _address[colons * 2] = acc >> 8;
+            _address[colons * 2 + 1] = acc & 0xff;
             colons++;
             acc = 0;
         }
@@ -233,15 +208,15 @@ bool IPAddress::fromString6(const char *address) {
         // Too many segments (double colon must be at least one zero field)
         return false;
     }
-    _address.bytes[colons * 2] = acc >> 8;
-    _address.bytes[colons * 2 + 1] = acc & 0xff;
+    _address[colons * 2] = acc >> 8;
+    _address[colons * 2 + 1] = acc & 0xff;
     colons++;
 
     if (double_colons != -1) {
         for (int i = colons * 2 - double_colons * 2 - 1; i >= 0; i--)
-            _address.bytes[16 - colons * 2 + double_colons * 2 + i] = _address.bytes[double_colons * 2 + i];
+            _address[16 - colons * 2 + double_colons * 2 + i] = _address[double_colons * 2 + i];
         for (int i = double_colons * 2; i < 16 - colons * 2 + double_colons * 2; i++)
-            _address.bytes[i] = 0;
+            _address[i] = 0;
     }
 
     _type = IPv6;
@@ -252,8 +227,10 @@ IPAddress& IPAddress::operator=(const uint8_t *address)
 {
     // IPv4 only conversion from byte pointer
     _type = IPv4;
-    memset(_address.bytes, 0, sizeof(_address.bytes));
-    memcpy(&_address.bytes[IPADDRESS_V4_BYTES_INDEX], address, sizeof(uint32_t));
+
+    _address.fill(0);
+    std::copy(address, address + 4, &_address[IPADDRESS_V4_BYTES_INDEX]);
+
     return *this;
 }
 
@@ -268,35 +245,35 @@ IPAddress& IPAddress::operator=(uint32_t address)
     // IPv4 conversion
     // See note on conversion/comparison and uint32_t
     _type = IPv4;
-    memset(_address.bytes, 0, sizeof(_address.bytes));
-    _address.dword[IPADDRESS_V4_DWORD_INDEX] = address;
+    _address.fill(0);
+    uint32_t& addressRef = reinterpret_cast<uint32_t&>(_address[IPADDRESS_V4_BYTES_INDEX]);
+    addressRef = address;
     return *this;
 }
 
 bool IPAddress::operator==(const IPAddress& addr) const {
-    return (addr._type == _type)
-        && (memcmp(addr._address.bytes, _address.bytes, sizeof(_address.bytes)) == 0);
+    return addr._type == _type && std::equal(addr._address.begin(), addr._address.end(), _address.begin());
 }
 
 bool IPAddress::operator==(const uint8_t* addr) const
 {
     // IPv4 only comparison to byte pointer
     // Can't support IPv6 as we know our type, but not the length of the pointer
-    return _type == IPv4 && memcmp(addr, &_address.bytes[IPADDRESS_V4_BYTES_INDEX], sizeof(uint32_t)) == 0;
+    return _type == IPv4 && std::equal(_address.begin(), _address.end(), addr);
 }
 
 uint8_t IPAddress::operator[](int index) const {
     if (_type == IPv4) {
-        return _address.bytes[IPADDRESS_V4_BYTES_INDEX + index];
+        return _address[IPADDRESS_V4_BYTES_INDEX + index];
     }
-    return _address.bytes[index];
+    return _address[index];
 }
 
 uint8_t& IPAddress::operator[](int index) {
     if (_type == IPv4) {
-        return _address.bytes[IPADDRESS_V4_BYTES_INDEX + index];
+        return _address[IPADDRESS_V4_BYTES_INDEX + index];
     }
-    return _address.bytes[index];
+    return _address[index];
 }
 
 size_t IPAddress::printTo(Print& p) const
@@ -310,7 +287,7 @@ size_t IPAddress::printTo(Print& p) const
         int8_t current_start = -1;
         int8_t current_length = 0;
         for (int8_t f = 0; f < 8; f++) {
-            if (_address.bytes[f * 2] == 0 && _address.bytes[f * 2 + 1] == 0) {
+            if (_address[f * 2] == 0 && _address[f * 2 + 1] == 0) {
                 if (current_start == -1) {
                     current_start = f;
                     current_length = 1;
@@ -327,10 +304,10 @@ size_t IPAddress::printTo(Print& p) const
         }
         for (int f = 0; f < 8; f++) {
             if (f < longest_start || f >= longest_start + longest_length) {
-                uint8_t c1 = _address.bytes[f * 2] >> 4;
-                uint8_t c2 = _address.bytes[f * 2] & 0xf;
-                uint8_t c3 = _address.bytes[f * 2 + 1] >> 4;
-                uint8_t c4 = _address.bytes[f * 2 + 1] & 0xf;
+                uint8_t c1 = _address[f * 2] >> 4;
+                uint8_t c2 = _address[f * 2] & 0xf;
+                uint8_t c3 = _address[f * 2 + 1] >> 4;
+                uint8_t c4 = _address[f * 2 + 1] & 0xf;
                 if (c1 > 0) {
                     n += p.print((char)(c1 < 10 ? '0' + c1 : 'a' + c1 - 10));
                 }
@@ -357,10 +334,10 @@ size_t IPAddress::printTo(Print& p) const
     // IPv4
     for (int i =0; i < 3; i++)
     {
-        n += p.print(_address.bytes[IPADDRESS_V4_BYTES_INDEX + i], DEC);
+        n += p.print(_address[IPADDRESS_V4_BYTES_INDEX + i], DEC);
         n += p.print('.');
     }
-    n += p.print(_address.bytes[IPADDRESS_V4_BYTES_INDEX + 3], DEC);
+    n += p.print(_address[IPADDRESS_V4_BYTES_INDEX + 3], DEC);
     return n;
 }
 

api/IPAddress.h

@@ -19,12 +19,12 @@
 
 #pragma once
 
+#include <array>
 #include <stdint.h>
 #include "Printable.h"
 #include "String.h"
 
 #define IPADDRESS_V4_BYTES_INDEX 12
-#define IPADDRESS_V4_DWORD_INDEX 3
 
 // forward declarations of global name space friend classes
 class EthernetClass;
@@ -42,17 +42,14 @@ enum IPType {
 
 class IPAddress : public Printable {
 private:
-    union {
-        uint8_t bytes[16];
-        uint32_t dword[4];
-    } _address;
-    IPType _type;
+    alignas(alignof(uint32_t)) std::array<uint8_t, 16> _address{};
+    IPType _type{IPv4};
 
     // Access the raw byte array containing the address.  Because this returns a pointer
     // to the internal structure rather than a copy of the address this function should only
     // be used when you know that the usage of the returned uint8_t* will be transient and not
     // stored.
-    uint8_t* raw_address() { return _type == IPv4 ? &_address.bytes[IPADDRESS_V4_BYTES_INDEX] : _address.bytes; }
+    uint8_t* raw_address() { return _type == IPv4 ? &_address[IPADDRESS_V4_BYTES_INDEX] : _address.data(); }
 
 public:
     // Constructors
@@ -75,7 +72,7 @@ class IPAddress : public Printable {
 
     // Overloaded cast operator to allow IPAddress objects to be used where a uint32_t is expected
     // NOTE: IPv4 only; see implementation note
-    operator uint32_t() const { return _type == IPv4 ? _address.dword[IPADDRESS_V4_DWORD_INDEX] : 0; };
+    operator uint32_t() const { return _type == IPv4 ? *reinterpret_cast<const uint32_t*>(&_address[IPADDRESS_V4_BYTES_INDEX]) : 0; };
 
     bool operator==(const IPAddress& addr) const;
     bool operator!=(const IPAddress& addr) const { return !(*this == addr); };
@@ -119,4 +116,4 @@ extern const IPAddress IN6ADDR_ANY;
 extern const IPAddress INADDR_NONE;
 }
 
-using arduino::IPAddress;
+using arduino::IPAddress;