From 91ec55b48c176e9c3901984631c3b5c368152a84 Mon Sep 17 00:00:00 2001 From: safocl Date: Fri, 3 Nov 2023 13:07:02 +0400 Subject: [PATCH] Rewrite IPAddress without union - Using a union allowed undefined behavior for array-like INPUT and OUTPUT arguments through a 32-bit integer (and vice versa). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- cores/esp32/IPAddress.cpp | 46 +++++++++++++++++++-------------------- cores/esp32/IPAddress.h | 19 ++++++++-------- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/cores/esp32/IPAddress.cpp b/cores/esp32/IPAddress.cpp index 0575363f254..e1b8a2b9f1d 100644 --- a/cores/esp32/IPAddress.cpp +++ b/cores/esp32/IPAddress.cpp @@ -20,62 +20,62 @@ #include #include #include +#include -IPAddress::IPAddress() -{ - _address.dword = 0; -} +// IPAddress::IPAddress() +// { +// _address.dword = 0; +// } IPAddress::IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet) -{ - _address.bytes[0] = first_octet; - _address.bytes[1] = second_octet; - _address.bytes[2] = third_octet; - _address.bytes[3] = fourth_octet; -} + : _address({first_octet, + second_octet, + third_octet, + fourth_octet}) +{} IPAddress::IPAddress(uint32_t address) { - _address.dword = address; + uint32_t& addressRef = reinterpret_cast(_address.front()); + addressRef = address; } -IPAddress::IPAddress(const uint8_t *address) -{ - memcpy(_address.bytes, address, sizeof(_address.bytes)); -} +IPAddress::IPAddress(const uint8_t *address) : _address({address[0], address[1], address[2], address[3]}) +{} IPAddress& IPAddress::operator=(const uint8_t *address) { - memcpy(_address.bytes, address, sizeof(_address.bytes)); + std::copy(address, address + _address.size(), _address.begin()); return *this; } IPAddress& IPAddress::operator=(uint32_t address) { - _address.dword = address; + uint32_t& addressRef = reinterpret_cast(_address.front()); + addressRef = address; return *this; } bool IPAddress::operator==(const uint8_t* addr) const { - return memcmp(addr, _address.bytes, sizeof(_address.bytes)) == 0; + return std::equal(_address.begin(), _address.end(), addr); } size_t IPAddress::printTo(Print& p) const { size_t n = 0; for(int i = 0; i < 3; i++) { - n += p.print(_address.bytes[i], DEC); + n += p.print(_address[i], DEC); n += p.print('.'); } - n += p.print(_address.bytes[3], DEC); + n += p.print(_address[3], DEC); return n; } String IPAddress::toString() const { char szRet[16]; - sprintf(szRet,"%u.%u.%u.%u", _address.bytes[0], _address.bytes[1], _address.bytes[2], _address.bytes[3]); + sprintf(szRet,"%u.%u.%u.%u", _address[0], _address[1], _address[2], _address[3]); return String(szRet); } @@ -103,7 +103,7 @@ bool IPAddress::fromString(const char *address) // Too much dots (there must be 3 dots) return false; } - _address.bytes[dots++] = acc; + _address[dots++] = acc; acc = 0; } else @@ -117,7 +117,7 @@ bool IPAddress::fromString(const char *address) // Too few dots (there must be 3 dots) return false; } - _address.bytes[3] = acc; + _address[3] = acc; return true; } diff --git a/cores/esp32/IPAddress.h b/cores/esp32/IPAddress.h index 3bedd4f8749..c0ebf225d20 100644 --- a/cores/esp32/IPAddress.h +++ b/cores/esp32/IPAddress.h @@ -23,16 +23,14 @@ #include #include #include +#include // A class to make it easier to handle and pass around IP addresses class IPAddress: public Printable { private: - union { - uint8_t bytes[4]; // IPv4 address - uint32_t dword; - } _address; + alignas(alignof(uint32_t)) std::array _address{}; // IPv4 address // 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 @@ -40,12 +38,12 @@ class IPAddress: public Printable // stored. uint8_t* raw_address() { - return _address.bytes; + return _address.data(); } public: // Constructors - IPAddress(); + IPAddress() = default; IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet); IPAddress(uint32_t address); IPAddress(const uint8_t *address); @@ -54,26 +52,27 @@ class IPAddress: public Printable bool fromString(const char *address); bool fromString(const String &address) { return fromString(address.c_str()); } + // Overloaded cast operator to allow IPAddress objects to be used where a pointer // to a four-byte uint8_t array is expected operator uint32_t() const { - return _address.dword; + return reinterpret_cast(_address.front()); } bool operator==(const IPAddress& addr) const { - return _address.dword == addr._address.dword; + return _address == addr._address; } bool operator==(const uint8_t* addr) const; // Overloaded index operator to allow getting and setting individual octets of the address uint8_t operator[](int index) const { - return _address.bytes[index]; + return _address[index]; } uint8_t& operator[](int index) { - return _address.bytes[index]; + return _address[index]; } // Overloaded copy operators to allow initialisation of IPAddress objects from other types