Use a union in IPAddress for uint8_t[] <-> uint32_t conversion

matthijskooijman · matthijskooijman · commit 3035239a4ee0 · 2014-02-19T16:09:29.000+01:00
Previously, pointer casting was used, but this resulted in strict-aliasing warnings: IPAddress.h: In member function ‘IPAddress::operator uint32_t() const’: IPAddress.h:46:61: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] operator uint32_t() const { return *((uint32_t*)_address); }; ^ IPAddress.h: In member function ‘bool IPAddress::operator==(const IPAddress&) const’: IPAddress.h:47:81: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); }; ^ IPAddress.h:47:114: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); }; Converting between unrelated types like this is commonly done using a union, which do not break the strict-aliasing rules. Using that union, inside IPAddress there is now an attribute _address.bytes for the raw byte arra, or _address.dword for the uint32_t version. Since we now have easy access to the uint32_t version, this also removes two memcpy invocations that can just become assignments. This patch does not change the generated code in any way, the compiler already optimized away the memcpy calls and the previous casts mean exactly the same. This is a different implementation of a part of arduino#1399 and it helps toward fixing arduino#1728.
diff --git a/hardware/arduino/avr/cores/arduino/IPAddress.cpp b/hardware/arduino/avr/cores/arduino/IPAddress.cpp
@@ -22,53 +22,53 @@
 
 IPAddress::IPAddress()
 {
-    memset(_address, 0, sizeof(_address));
+    _address.dword = 0;
 }
 
 IPAddress::IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet)
 {
-    _address[0] = first_octet;
-    _address[1] = second_octet;
-    _address[2] = third_octet;
-    _address[3] = fourth_octet;
+    _address.bytes[0] = first_octet;
+    _address.bytes[1] = second_octet;
+    _address.bytes[2] = third_octet;
+    _address.bytes[3] = fourth_octet;
 }
 
 IPAddress::IPAddress(uint32_t address)
 {
-    memcpy(_address, &address, sizeof(_address));
+    _address.dword = address;
 }
 
 IPAddress::IPAddress(const uint8_t *address)
 {
-    memcpy(_address, address, sizeof(_address));
+    memcpy(_address.bytes, address, sizeof(_address.bytes));
 }
 
 IPAddress& IPAddress::operator=(const uint8_t *address)
 {
-    memcpy(_address, address, sizeof(_address));
+    memcpy(_address.bytes, address, sizeof(_address.bytes));
     return *this;
 }
 
 IPAddress& IPAddress::operator=(uint32_t address)
 {
-    memcpy(_address, (const uint8_t *)&address, sizeof(_address));
+    _address.dword = address;
     return *this;
 }
 
 bool IPAddress::operator==(const uint8_t* addr) const
 {
-    return memcmp(addr, _address, sizeof(_address)) == 0;
+    return memcmp(addr, _address.bytes, sizeof(_address.bytes)) == 0;
 }
 
 size_t IPAddress::printTo(Print& p) const
 {
     size_t n = 0;
     for (int i =0; i < 3; i++)
     {
-        n += p.print(_address[i], DEC);
+        n += p.print(_address.bytes[i], DEC);
         n += p.print('.');
     }
-    n += p.print(_address[3], DEC);
+    n += p.print(_address.bytes[3], DEC);
     return n;
 }
 
diff --git a/hardware/arduino/avr/cores/arduino/IPAddress.h b/hardware/arduino/avr/cores/arduino/IPAddress.h
@@ -27,12 +27,16 @@
 
 class IPAddress : public Printable {
 private:
-    uint8_t _address[4];  // IPv4 address
+    union {
+	uint8_t bytes[4];  // IPv4 address
+	uint32_t dword;
+    } _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
     // be used when you know that the usage of the returned uint8_t* will be transient and not
     // stored.
-    uint8_t* raw_address() { return _address; };
+    uint8_t* raw_address() { return _address.bytes; };
 
 public:
     // Constructors
@@ -43,13 +47,13 @@ class IPAddress : public Printable {
 
     // 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 *((uint32_t*)_address); };
-    bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); };
+    operator uint32_t() const { return _address.dword; };
+    bool operator==(const IPAddress& addr) const { return _address.dword == addr._address.dword; };
     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[index]; };
-    uint8_t& operator[](int index) { return _address[index]; };
+    uint8_t operator[](int index) const { return _address.bytes[index]; };
+    uint8_t& operator[](int index) { return _address.bytes[index]; };
 
     // Overloaded copy operators to allow initialisation of IPAddress objects from other types
     IPAddress& operator=(const uint8_t *address);
diff --git a/hardware/arduino/sam/cores/arduino/IPAddress.cpp b/hardware/arduino/sam/cores/arduino/IPAddress.cpp
@@ -22,53 +22,53 @@
 
 IPAddress::IPAddress()
 {
-    memset(_address, 0, sizeof(_address));
+    _address.dword = 0;
 }
 
 IPAddress::IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet)
 {
-    _address[0] = first_octet;
-    _address[1] = second_octet;
-    _address[2] = third_octet;
-    _address[3] = fourth_octet;
+    _address.bytes[0] = first_octet;
+    _address.bytes[1] = second_octet;
+    _address.bytes[2] = third_octet;
+    _address.bytes[3] = fourth_octet;
 }
 
 IPAddress::IPAddress(uint32_t address)
 {
-    memcpy(_address, &address, sizeof(_address));
+    _address.dword = address;
 }
 
 IPAddress::IPAddress(const uint8_t *address)
 {
-    memcpy(_address, address, sizeof(_address));
+    memcpy(_address.bytes, address, sizeof(_address.bytes));
 }
 
 IPAddress& IPAddress::operator=(const uint8_t *address)
 {
-    memcpy(_address, address, sizeof(_address));
+    memcpy(_address.bytes, address, sizeof(_address.bytes));
     return *this;
 }
 
 IPAddress& IPAddress::operator=(uint32_t address)
 {
-    memcpy(_address, (const uint8_t *)&address, sizeof(_address));
+    _address.dword = address;
     return *this;
 }
 
 bool IPAddress::operator==(const uint8_t* addr) const
 {
-    return memcmp(addr, _address, sizeof(_address)) == 0;
+    return memcmp(addr, _address.bytes, sizeof(_address.bytes)) == 0;
 }
 
 size_t IPAddress::printTo(Print& p) const
 {
     size_t n = 0;
     for (int i =0; i < 3; i++)
     {
-        n += p.print(_address[i], DEC);
+        n += p.print(_address.bytes[i], DEC);
         n += p.print('.');
     }
-    n += p.print(_address[3], DEC);
+    n += p.print(_address.bytes[3], DEC);
     return n;
 }
 
diff --git a/hardware/arduino/sam/cores/arduino/IPAddress.h b/hardware/arduino/sam/cores/arduino/IPAddress.h
@@ -27,12 +27,16 @@
 
 class IPAddress : public Printable {
 private:
-    uint8_t _address[4];  // IPv4 address
+    union {
+	uint8_t bytes[4];  // IPv4 address
+	uint32_t dword;
+    } _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
     // be used when you know that the usage of the returned uint8_t* will be transient and not
     // stored.
-    uint8_t* raw_address() { return _address; };
+    uint8_t* raw_address() { return _address.bytes; };
 
 public:
     // Constructors
@@ -43,13 +47,13 @@ class IPAddress : public Printable {
 
     // 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 *((uint32_t*)_address); };
-    bool operator==(const IPAddress& addr) const { return (*((uint32_t*)_address)) == (*((uint32_t*)addr._address)); };
+    operator uint32_t() const { return _address.dword; };
+    bool operator==(const IPAddress& addr) const { return _address.dword == addr._address.dword; };
     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[index]; };
-    uint8_t& operator[](int index) { return _address[index]; };
+    uint8_t operator[](int index) const { return _address.bytes[index]; };
+    uint8_t& operator[](int index) { return _address.bytes[index]; };
 
     // Overloaded copy operators to allow initialisation of IPAddress objects from other types
     IPAddress& operator=(const uint8_t *address);

Original file line number	Diff line number	Diff line change
`@@ -22,53 +22,53 @@`
`22`	`22`
`23`	`23`	`IPAddress::IPAddress()`
`24`	`24`	`{`
`25`		`- memset(_address, 0, sizeof(_address));`
	`25`	`+ _address.dword = 0;`
`26`	`26`	`}`
`27`	`27`
`28`	`28`	`IPAddress::IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet)`
`29`	`29`	`{`
`30`		`- _address[0] = first_octet;`
`31`		`- _address[1] = second_octet;`
`32`		`- _address[2] = third_octet;`
`33`		`- _address[3] = fourth_octet;`
	`30`	`+ _address.bytes[0] = first_octet;`
	`31`	`+ _address.bytes[1] = second_octet;`
	`32`	`+ _address.bytes[2] = third_octet;`
	`33`	`+ _address.bytes[3] = fourth_octet;`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`IPAddress::IPAddress(uint32_t address)`
`37`	`37`	`{`
`38`		`- memcpy(_address, &address, sizeof(_address));`
	`38`	`+ _address.dword = address;`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`IPAddress::IPAddress(const uint8_t *address)`
`42`	`42`	`{`
`43`		`- memcpy(_address, address, sizeof(_address));`
	`43`	`+ memcpy(_address.bytes, address, sizeof(_address.bytes));`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`IPAddress& IPAddress::operator=(const uint8_t *address)`
`47`	`47`	`{`
`48`		`- memcpy(_address, address, sizeof(_address));`
	`48`	`+ memcpy(_address.bytes, address, sizeof(_address.bytes));`
`49`	`49`	`return *this;`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`IPAddress& IPAddress::operator=(uint32_t address)`
`53`	`53`	`{`
`54`		`- memcpy(_address, (const uint8_t *)&address, sizeof(_address));`
	`54`	`+ _address.dword = address;`
`55`	`55`	`return *this;`
`56`	`56`	`}`
`57`	`57`
`58`	`58`	`bool IPAddress::operator==(const uint8_t* addr) const`
`59`	`59`	`{`
`60`		`- return memcmp(addr, _address, sizeof(_address)) == 0;`
	`60`	`+ return memcmp(addr, _address.bytes, sizeof(_address.bytes)) == 0;`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`size_t IPAddress::printTo(Print& p) const`
`64`	`64`	`{`
`65`	`65`	`size_t n = 0;`
`66`	`66`	`for (int i =0; i < 3; i++)`
`67`	`67`	`{`
`68`		`- n += p.print(_address[i], DEC);`
	`68`	`+ n += p.print(_address.bytes[i], DEC);`
`69`	`69`	`n += p.print('.');`
`70`	`70`	`}`
`71`		`- n += p.print(_address[3], DEC);`
	`71`	`+ n += p.print(_address.bytes[3], DEC);`
`72`	`72`	`return n;`
`73`	`73`	`}`
`74`	`74`