Skip to content

Commit b7eaa07

Browse files
committed
refactor: adaptation for discussion
some code refactored (`placement-new` rather than `memcpy`; `raw_address` rather than `&_address[IPADDRESS_V4_BYTES_INDEX]`). other parts of the C++ standard have been added and embedded into the code for discussion.
1 parent dc7b5b5 commit b7eaa07

File tree

2 files changed

+50
-22
lines changed

2 files changed

+50
-22
lines changed

api/IPAddress.cpp

+44-20
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
#include "IPAddress.h"
2121
#include "Print.h"
22+
#include <cstdint>
23+
#include <new>
2224

2325
using namespace arduino;
2426

@@ -39,9 +41,40 @@ IPAddress::IPAddress(uint8_t o1, uint8_t o2, uint8_t o3, uint8_t o4, uint8_t o5,
3941
// IPv4 only
4042
IPAddress::IPAddress(uint32_t address)
4143
{
42-
memcpy(&_address[IPADDRESS_V4_BYTES_INDEX], &address, 4); // This method guarantees a defined behavior. Any pointer conversions to write to ADDRESS storage (as a multibyte integer) are undefined behavior when the lifetime of the multibyte type has not previously started.
43-
44-
// C++ standard draft [basic.life#7](https://eel.is/c++draft/basic.life#7)
44+
// memcpy(raw_address(), &address, 4); // This method guarantees a defined behavior.
45+
// But lifetime started when:
46+
// [basic.life#2] https://eel.is/c++draft/basic.life#2
47+
//(2.1) -- storage with the proper alignment and size for type T is obtained, and
48+
//(2.2) -- its initialization (if any) is complete (including vacuous initialization) ([dcl.init]),
49+
//
50+
// The statement: {#Any pointer conversions to write to ADDRESS storage (as a multibyte integer)
51+
// are undefined behavior when the lifetime of the multibyte type has not previously started.#}
52+
// only apply to c++17 and earlier. Since C++20 array of bytes implicitly creates the inner objects.
53+
54+
// C++20: https://timsong-cpp.github.io/cppwp/n4861/intro.object#13
55+
// 13 An operation that begins the lifetime of an array of char, unsigned char, or std::byte implicitly creates objects within
56+
// the region of storage occupied by the array. [ Note: The array object provides storage for these objects. — end note ]
57+
58+
// C++23: https://timsong-cpp.github.io/cppwp/n4950/intro.object#13
59+
// 13 An operation that begins the lifetime of an array of unsigned char or std::byte implicitly creates objects within the
60+
// region of storage occupied by the array.
61+
// [Note 5: The array object provides storage for these objects. — end note]
62+
63+
// Current draft: https://eel.is/c++draft/intro.object#14
64+
// 14 Except during constant evaluation, an operation that begins the lifetime of an array of unsigned char or std::byte implicitly
65+
// creates objects within the region of storage occupied by the array.
66+
// [Note 5: The array object provides storage for these objects. — end note]
67+
68+
// *reinterpret_cast<uint32_t*>(_address[IPADDRESS_V4_BYTES_INDEX]) = address; // This valid initialization in the `_address` storage since C++20.
69+
// now the pointer `_address[IPADDRESS_V4_BYTES_INDEX]` points to a multibyte int.
70+
71+
new (&_address[IPADDRESS_V4_BYTES_INDEX]) uint32_t (address); // But the new-expression is better for understanding and looks nicer (for trivial types, the
72+
// new expression only begins its lifetime and does not perform any other actions).
73+
// NOTE: https://en.cppreference.com/w/cpp/language/new#Notes
74+
75+
// NOTE: new-expression and reinterpret_cast require alignment of the storage, but memcpy does not.
76+
77+
// C++ standard draft [basic.life#7](https://eel.is/c++draft/basic.life#7)
4578
// Before the lifetime of an object has started but after the storage which the object
4679
// will occupy has been allocated or, after the lifetime of an object has ended and
4780
// before the storage which the object occupied is reused or released, any pointer that
@@ -57,7 +90,7 @@ IPAddress::IPAddress(uint32_t address)
5790
// when the conversion is to pointer to cv void, or to pointer to cv void and subsequently
5891
// to pointer to cv char, cv unsigned char, or cv std::byte ([cstddef.syn]), or
5992

60-
// C++ standard draft [basic.life#8](https://eel.is/c++draft/basic.life#8)
93+
// C++ standard draft [basic.life#8](https://eel.is/c++draft/basic.life#8)
6194
// Similarly, before the lifetime of an object has started but after the storage which
6295
// the object will occupy has been allocated or, after the lifetime of an object has
6396
// ended and before the storage which the object occupied is reused or released, any
@@ -81,11 +114,8 @@ IPAddress::IPAddress(const uint8_t *address) : IPAddress(IPv4, address) {}
81114

82115
IPAddress::IPAddress(IPType ip_type, const uint8_t *address) : _type(ip_type)
83116
{
84-
if (ip_type == IPv4) {
85-
memcpy(&_address[IPADDRESS_V4_BYTES_INDEX], address, sizeof(uint32_t));
86-
} else {
87-
memcpy(_address, address, sizeof(_address));
88-
}
117+
const size_t copy_size = (ip_type == IPv4) ? sizeof(uint32_t) : sizeof(_address);
118+
memcpy(raw_address(), address, sizeof(_address));
89119
}
90120

91121
IPAddress::IPAddress(const char *address)
@@ -253,7 +283,7 @@ IPAddress& IPAddress::operator=(const uint8_t *address)
253283
_type = IPv4;
254284

255285
memset(_address, 0, sizeof(_address));
256-
memcpy(&_address[IPADDRESS_V4_BYTES_INDEX], address, sizeof(uint32_t));
286+
memcpy(raw_address(), address, sizeof(uint32_t));
257287

258288
return *this;
259289
}
@@ -270,7 +300,7 @@ IPAddress& IPAddress::operator=(uint32_t address)
270300
// See note on conversion/comparison and uint32_t
271301
_type = IPv4;
272302
memset(_address, 0, sizeof(_address));
273-
memcpy(&_address[IPADDRESS_V4_BYTES_INDEX], &address, 4);
303+
new (raw_address()) uint32_t (address); // See the comments in corresponding constructor.
274304
return *this;
275305
}
276306

@@ -283,21 +313,15 @@ bool IPAddress::operator==(const uint8_t* addr) const
283313
{
284314
// IPv4 only comparison to byte pointer
285315
// Can't support IPv6 as we know our type, but not the length of the pointer
286-
return _type == IPv4 && memcmp(addr, &_address[IPADDRESS_V4_BYTES_INDEX], sizeof(uint32_t)) == 0;
316+
return _type == IPv4 && memcmp(addr, raw_address(), sizeof(uint32_t)) == 0;
287317
}
288318

289319
uint8_t IPAddress::operator[](int index) const {
290-
if (_type == IPv4) {
291-
return _address[IPADDRESS_V4_BYTES_INDEX + index];
292-
}
293-
return _address[index];
320+
return *(raw_address() + index);
294321
}
295322

296323
uint8_t& IPAddress::operator[](int index) {
297-
if (_type == IPv4) {
298-
return _address[IPADDRESS_V4_BYTES_INDEX + index];
299-
}
300-
return _address[index];
324+
return *(raw_address() + index);
301325
}
302326

303327
size_t IPAddress::printTo(Print& p) const

api/IPAddress.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ class IPAddress : public Printable {
5252
// to the internal structure rather than a copy of the address this function should only
5353
// be used when you know that the usage of the returned uint8_t* will be transient and not
5454
// stored.
55+
const uint8_t* raw_address() const { return _type == IPv4 ? &_address[IPADDRESS_V4_BYTES_INDEX] : _address; }
5556
uint8_t* raw_address() { return _type == IPv4 ? &_address[IPADDRESS_V4_BYTES_INDEX] : _address; }
57+
// NOTE: If IPADDRESS_V4_BYTES_INDEX region can be initialized with a multibyte int, then a cast to unsigned char is required.
58+
5659

5760
public:
5861
// Constructors
@@ -80,8 +83,9 @@ class IPAddress : public Printable {
8083
// The user is responsible for ensuring that the value is converted to BigEndian.
8184
operator uint32_t() const {
8285
uint32_t ret;
83-
memcpy(&ret, &_address[IPADDRESS_V4_BYTES_INDEX], 4);
84-
// NOTE: maybe use the placement-new for starting of the integer type lifetime in the storage when constructing an IPAddress?
86+
memcpy(&ret, raw_address(), 4);
87+
// NOTE: maybe use the placement-new (or standart initialisation of uint32_t since C++20)
88+
// for starting of the integer type lifetime in the storage when constructing an IPAddress?
8589
// FIXME: need endianness checking? how do this with the arduino-api?
8690
return _type == IPv4 ? ret : 0;
8791
};

0 commit comments

Comments
 (0)