Skip to content

Commit

Permalink
rewrite(IPAddress): fix implementation
Browse files Browse the repository at this point in the history
fix implementation and add discussion questions (safe using of
IPAddress).
  • Loading branch information
safocl committed Jan 31, 2025
1 parent d85523f commit 77f7375
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
33 changes: 29 additions & 4 deletions api/IPAddress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,34 @@ IPAddress::IPAddress(uint8_t o1, uint8_t o2, uint8_t o3, uint8_t o4, uint8_t o5,
// IPv4 only
IPAddress::IPAddress(uint32_t address)
{
uint32_t& addressRef = reinterpret_cast<uint32_t&>(_address[IPADDRESS_V4_BYTES_INDEX]);
addressRef = address;
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.

// C++ standard draft [basic.life#7](https://eel.is/c++draft/basic.life#7)
// Before the lifetime of an object has started but after the storage which the object
// will occupy has been allocated or, after the lifetime of an object has ended and
// before the storage which the object occupied is reused or released, any pointer that
// represents the address of the storage location where the object will be or was
// located may be used but only in limited ways. For an object under construction or
// destruction, see [class.cdtor]. Otherwise, such a pointer refers to allocated storage
// ([basic.stc.dynamic.allocation]), and using the pointer as if the pointer were of
// type void* is well-defined. Indirection through such a pointer is permitted but the
// resulting lvalue may only be used in limited ways, as described below.
// The program has undefined behavior if
// --the pointer is used as the operand of a delete-expression,
// --the pointer is used as the operand of a static_cast ([expr.static.cast]), except
// when the conversion is to pointer to cv void, or to pointer to cv void and subsequently
// to pointer to cv char, cv unsigned char, or cv std::byte ([cstddef.syn]), or

// C++ standard draft [basic.life#8](https://eel.is/c++draft/basic.life#8)
// Similarly, before the lifetime of an object has started but after the storage which
// the object will occupy has been allocated or, after the lifetime of an object has
// ended and before the storage which the object occupied is reused or released, any
// glvalue that refers to the original object may be used but only in limited ways.
// For an object under construction or destruction, see [class.cdtor]. Otherwise, such
// a glvalue refers to allocated storage ([basic.stc.dynamic.allocation]), and using
// the properties of the glvalue that do not depend on its value is well-defined.
// The program has undefined behavior if
// -- the glvalue is used to access the object, or

// NOTE on conversion/comparison and uint32_t:
// These conversions are host platform dependent.
Expand Down Expand Up @@ -244,8 +270,7 @@ IPAddress& IPAddress::operator=(uint32_t address)
// See note on conversion/comparison and uint32_t
_type = IPv4;
memset(_address, 0, sizeof(_address));
uint32_t& addressRef = reinterpret_cast<uint32_t&>(_address[IPADDRESS_V4_BYTES_INDEX]);
addressRef = address;
memcpy(&_address[IPADDRESS_V4_BYTES_INDEX], &address, 4);
return *this;
}

Expand Down
17 changes: 15 additions & 2 deletions api/IPAddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#pragma once

#include <string.h>
#include <stdint.h>
#include "Printable.h"
#include "String.h"
Expand All @@ -41,7 +42,10 @@ enum IPType {

class IPAddress : public Printable {
private:
alignas(alignof(uint32_t)) uint8_t _address[16]{};
alignas(alignof(uint32_t)) uint8_t _address[16]{}; // If the implementation does not require
// storage as a multibyte integer, you can
// remove the storage field alignment.
// Address (as uint32) is accessed by copying.
IPType _type{IPv4};

// Access the raw byte array containing the address. Because this returns a pointer
Expand All @@ -59,6 +63,7 @@ class IPAddress : public Printable {
IPAddress(uint8_t first_octet, uint8_t second_octet, uint8_t third_octet, uint8_t fourth_octet);
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);
// IPv4; see implementation note
// NOTE: address MUST BE BigEndian.
IPAddress(uint32_t address);
// Default IPv4
IPAddress(const uint8_t *address);
Expand All @@ -71,7 +76,15 @@ 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 ? *reinterpret_cast<const uint32_t*>(&_address[IPADDRESS_V4_BYTES_INDEX]) : 0; };
// NOTE: Data of the returned integer in the native endiannes, but relevant ordering is a BigEndian.

Check failure on line 79 in api/IPAddress.h

View workflow job for this annotation

GitHub Actions / spellcheck

endiannes ==> endianness
// The user is responsible for ensuring that the value is converted to BigEndian.
operator uint32_t() const {
uint32_t ret;
memcpy(&ret, &_address[IPADDRESS_V4_BYTES_INDEX], 4);
// NOTE: maybe use the placement-new for starting of the integer type lifetime in the storage when constructing an IPAddress?
// FIXME: need endiannes checking? how do this with the arduino-api?

Check failure on line 85 in api/IPAddress.h

View workflow job for this annotation

GitHub Actions / spellcheck

endiannes ==> endianness
return _type == IPv4 ? ret : 0;
};

bool operator==(const IPAddress& addr) const;
bool operator!=(const IPAddress& addr) const { return !(*this == addr); };
Expand Down

0 comments on commit 77f7375

Please sign in to comment.