From f76c327aae447ca595a428c6783d817ec2f23ed9 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Tue, 18 Feb 2014 18:11:22 +0100
Subject: Use a union in IPAddress for uint8_t[] <-> uint32_t conversion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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 #1399 and it helps
toward fixing #1728.
---
cores/arduino/IPAddress.cpp | 24 ++++++++++++------------
cores/arduino/IPAddress.h | 16 ++++++++++------
2 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/cores/arduino/IPAddress.cpp b/cores/arduino/IPAddress.cpp
index 22a0e42..899cbd4 100644
--- a/cores/arduino/IPAddress.cpp
+++ b/cores/arduino/IPAddress.cpp
@@ -22,42 +22,42 @@
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
@@ -65,10 +65,10 @@ 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/cores/arduino/IPAddress.h b/cores/arduino/IPAddress.h
index 6ea8127..94acdc4 100644
--- a/cores/arduino/IPAddress.h
+++ b/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 @@ public:
// 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);
--
cgit v1.2.3-18-g5258
From a2408d154eb9fa3fa9b07cc5a04e9b3744a9f81a Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Tue, 18 Feb 2014 19:34:04 +0100
Subject: Instead of #defining true and false, include stdbool.h
In C++, true and false are language keywords, so there is no need to
define them as macros. Including stdbool.h in C++ effectively changes
nothing. In C, true, false and also the bool type are not available, but
including stdbool.h will make them available.
Using stdbool.h means that we get true, false and the bool type in
whatever way the compiler thinks is best, which seems like a good idea
to me.
This also fixes the following compiler warnings if a .c file includes
both stdbool.h and Arduino.h:
warning: "true" redefined [enabled by default]
#define true 0x1
warning: "false" redefined [enabled by default]
#define false 0x0
This fixes #1570 and helps toward fixing #1728.
This only changed the AVR core, the SAM core already doesn't define true
and false (but doesn't include stdbool.h either).
---
cores/arduino/Arduino.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/cores/arduino/Arduino.h b/cores/arduino/Arduino.h
index e12ac82..ec1389e 100644
--- a/cores/arduino/Arduino.h
+++ b/cores/arduino/Arduino.h
@@ -21,6 +21,7 @@
#define Arduino_h
#include <stdlib.h>
+#include <stdbool.h>
#include <string.h>
#include <math.h>
@@ -43,9 +44,6 @@ void yield(void);
#define OUTPUT 0x1
#define INPUT_PULLUP 0x2
-#define true 0x1
-#define false 0x0
-
#define PI 3.1415926535897932384626433832795
#define HALF_PI 1.5707963267948966192313216916398
#define TWO_PI 6.283185307179586476925286766559
--
cgit v1.2.3-18-g5258
From 53c0f1412d9a53ddc7bdeb1743d9054f552b1dab Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Tue, 18 Feb 2014 20:56:31 +0100
Subject: Don't store peeked characters in a char variable
peekNextDigit() returns an int, so it can return -1 in addition to all
256 possible bytes. By putting the result in a signe char, all bytes
over 128 will be interpreted as "no bytes available". Furthermore, it
seems that on SAM "char" is unsigned by default, causing the
"if (c < 0)" line a bit further down to always be false.
Using an int is more appropriate.
A different fix for this issue was suggested in #1399. This fix helps
towards #1728.
---
cores/arduino/Stream.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cores/arduino/Stream.cpp b/cores/arduino/Stream.cpp
index aafb7fc..a12a72e 100644
--- a/cores/arduino/Stream.cpp
+++ b/cores/arduino/Stream.cpp
@@ -176,7 +176,7 @@ float Stream::parseFloat(char skipChar){
boolean isNegative = false;
boolean isFraction = false;
long value = 0;
- char c;
+ int c;
float fraction = 1.0;
c = peekNextDigit();
--
cgit v1.2.3-18-g5258