Add at_address() to etl::unaligned_type

[rolandreichweinbmw]

I'm regularly involved with code like this:

uint8_t data[4];

*reinterpret_cast<etl::be_uint32_t*>(data) = 0x12345678;

uint32_t value = *reinterpret_cast<etl::be_uint32_t*>(data);

To encapsulate the reinterpret_cast, the at_address() is a convenience function to get to a reinterpreted etl::unaligned_type, e.g.

etl::be_uint32_t::at_address(data) = 0x12345678;

uint32_t value = etl::be_uint32_t::at_address(data);

[jwellbelove]

I think it would be useful to add overloads that include the buffer size, to allow the option for extra safety checks on the buffer size vs the type size..
Here are the functions that could be added, One pair have a run time size check (ETL_ASSERT) and the others have compile time size check (ETL_STATIC_ASSERT).

    // For run time pointer and size
    static unaligned_type<T, Endian_>& at_address(void* address, size_t buffer_size)
    {
      ETL_ASSERT(sizeof(T) <= buffer_size, ETL_ERROR(etl::unaligned_type_buffer_size));

      return *reinterpret_cast<unaligned_type<T, Endian_>*>(address);
    }

    // For const run time pointer and size
    static const unaligned_type<T, Endian_>& at_address(const void* address, size_t buffer_size)
    {
      ETL_ASSERT(sizeof(T) <= buffer_size, ETL_ERROR(etl::unaligned_type_buffer_size));

      return *reinterpret_cast<const unaligned_type<T, Endian_>*>(address);
    }

    // For run time pointer and compile time size
    template <size_t Size>
    static unaligned_type<T, Endian_>& at_address(void* address)
    {
      ETL_STATIC_ASSERT(sizeof(T) <= Size, "Buffer size to small for type");

      return *reinterpret_cast<unaligned_type<T, Endian_>*>(address);
    }

    // For const run time pointer and compile time size
    template <size_t Size>
    static unaligned_type<T, Endian_>& at_address(const void* address)
    {
      ETL_STATIC_ASSERT(sizeof(T) <= Size, "Buffer size to small for type");

      return *reinterpret_cast<const unaligned_type<T, Endian_>*>(address);
    }

It would require adding an exception type.

  struct unaligned_type_exception : public etl::exception
  {
  public:

    unaligned_type_exception(string_type reason_, string_type file_name_, numeric_type line_number_)
      : exception(reason_, file_name_, line_number_)
    {
    }
  };

  class unaligned_type_buffer_size : public unaligned_type_exception
  {
  public:

    unaligned_type_buffer_size(string_type file_name_, numeric_type line_number_)
      : unaligned_type_exception(ETL_ERROR_TEXT("unaligned_type:buffer size", ETL_UNALIGNED_TYPE_FILE_ID"A"), file_name_, line_number_)
    {
    }
  };

ETL_UNALIGNED_TYPE_FILE_ID would be defined in #include file_error_numbers.h

[rolandreichweinbmw]

I agree. I just added it to the PR.

[rolandreichweinbmw]

Now also added the unit tests.

[jwellbelove]

The only other thing that slightly bugs me is that I'm a little uncertain as to whether this breaks the 'strict aliasing' rules and do something weird in some circumstances in highly optimised code.
We are saying that an object of type unaligned_type lives at an address where one does not exist.

I used to do that all the time back when I coded in C and compilers were less able to do sophisticated optimisations, but I'm not so sure about modern C++ compilers.

If it is breaking strict aliasing, then I wonder whether an internal placement-new would more compliant?

The absolute safe solution would be to initialise an unaligned_type from an address/size with a new set of constructors. (I may add these anyway)

[rolandreichweinbmw]

In the initially described use case, you always end up reinterpreting data from inside another classes instance. This is true for currently implemented reinterpret_cast, or placement new, or a new class unaligned_type_ext.

If you mean new constructors for unaligned_type with copy-by-value, this would cover only the read, not the write.

class span suffers from the same dilemma, doesn't it?

[jwellbelove]

In the containers there are placement-new calls using a pointer to a char array, but the code outside of the container never sees the array, and once placement new occurs, the memory is always accessed as the container's type.

In etl::unaligned_type, the raw buffer and the overlaid unaligned_type reference may both have interleaved accessed by the code. But, according to the language rules, the compiler can assume that the buffer and the unaligned_type object are independent of each other and it's possible (I think) that optimisation could cause undefined things to happen.

As the raw buffer and internal storage of unaligned_type that aliases it are compatible types and there are no other member variables, then maybe there would be no problem. I not a big enough of an expert on this part of the language spec to say for definite.

If you mean new constructors for unaligned_type with copy-by-value, this would cover only the read, not the write.

It would just be another way of constructing an unaligned_type from a 'value'.

Another way to do this is to do the same as what I did with the _ext (external storage) containers.
An etl::unaligned_type_ext would be initialised with a pointer to it's external raw storage.
The usage of the class would be nearly identical, and would work for both read and write.

uint8_t data[] = {0x01, 0x02, 0x03, 0x04};
CHECK_EQUAL(etl::be_uint32_t_ext(data), 0x01020304);

It could be changed on the fly too.

uint8_t data1[] = {0x12, 0x34, 0x56, 0x78};
uint8_t data2[] = {0x78, 0x56, 0x34, 0x12};

etl::be_uint32_t_ext be_u32_data;

be_u32_data.set_storage(data1);
CHECK_EQUAL(be_u32_data, 0x12345678);

be_u32_data.set_storage(data2);
CHECK_EQUAL(be_u32_data, 0x78563412);

class span suffers from the same dilemma, doesn't it?

A span contains pointers to the actual type.

[rolandreichweinbmw]

I agree that an etl::unaligned_type_ext would be more consistent.