From 2f31267ea2ec0362e2f690a0a6a28609a75efc23 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Mon, 3 Oct 2022 01:00:51 +0100 Subject: [PATCH 1/3] char to unsigned char storage --- include/etl/unaligned_type.h | 100 ++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/include/etl/unaligned_type.h b/include/etl/unaligned_type.h index 54b72205..8a65f54c 100644 --- a/include/etl/unaligned_type.h +++ b/include/etl/unaligned_type.h @@ -58,10 +58,11 @@ namespace etl static ETL_CONSTANT size_t Size = Size_; - typedef char* pointer; - typedef const char* const_pointer; - typedef char* iterator; - typedef const char* const_iterator; + typedef unsigned char storage_type; + typedef storage_type* pointer; + typedef const storage_type* const_pointer; + typedef storage_type* iterator; + typedef const storage_type* const_iterator; typedef etl::reverse_iterator reverse_iterator; typedef etl::reverse_iterator const_reverse_iterator; @@ -196,7 +197,7 @@ namespace etl //************************************************************************* /// Index operator. //************************************************************************* - char& operator[](int i) + storage_type& operator[](int i) { return storage[i]; } @@ -204,14 +205,14 @@ namespace etl //************************************************************************* /// Const index operator. //************************************************************************* - ETL_CONSTEXPR const char& operator[](int i) const + ETL_CONSTEXPR const storage_type& operator[](int i) const { return storage[i]; } protected: - char storage[Size]; + unsigned char storage[Size]; }; } @@ -227,6 +228,9 @@ namespace etl typedef T value_type; + typedef typename private_unaligned_type::unaligned_type_common::pointer pointer; + typedef typename private_unaligned_type::unaligned_type_common::const_pointer const_pointer; + static ETL_CONSTANT int Endian = Endian_; static ETL_CONSTANT size_t Size = private_unaligned_type::unaligned_type_common::Size; @@ -362,19 +366,19 @@ namespace etl struct unaligned_copy { //******************************* - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, pointer store) { - store[0] = static_cast(value); + store[0] = static_cast(value); } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const_pointer store, T& value) { value = static_cast(store[0]); } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int /*endian_src*/, char* dst) + static ETL_CONSTEXPR14 void copy(const_pointer src, int /*endian_src*/, unsigned char* dst) { dst[0] = src[0]; } @@ -387,22 +391,22 @@ namespace etl struct unaligned_copy { //******************************* - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { if (Endian == etl::endianness::value()) { - store[0] = static_cast(value); - store[1] = static_cast(value >> (1U * CHAR_BIT)); + store[0] = static_cast(value); + store[1] = static_cast(value >> (1U * CHAR_BIT)); } else { - store[1] = static_cast(value); - store[0] = static_cast(value >> (1U * CHAR_BIT)); + store[1] = static_cast(value); + store[0] = static_cast(value >> (1U * CHAR_BIT)); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const_pointer store, T& value) { if (Endian == etl::endianness::value()) { @@ -417,7 +421,7 @@ namespace etl } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int endian_src, char* dst) + static ETL_CONSTEXPR14 void copy(const_pointer src, int endian_src, unsigned char* dst) { if (Endian == endian_src) { @@ -438,26 +442,26 @@ namespace etl template struct unaligned_copy { - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { if (Endian == etl::endianness::value()) { - store[0] = static_cast(value); - store[1] = static_cast(value >> (1U * CHAR_BIT)); - store[2] = static_cast(value >> (2U * CHAR_BIT)); - store[3] = static_cast(value >> (3U * CHAR_BIT)); + store[0] = static_cast(value); + store[1] = static_cast(value >> (1U * CHAR_BIT)); + store[2] = static_cast(value >> (2U * CHAR_BIT)); + store[3] = static_cast(value >> (3U * CHAR_BIT)); } else { - store[3] = static_cast(value); - store[2] = static_cast(value >> (1U * CHAR_BIT)); - store[1] = static_cast(value >> (2U * CHAR_BIT)); - store[0] = static_cast(value >> (3U * CHAR_BIT)); + store[3] = static_cast(value); + store[2] = static_cast(value >> (1U * CHAR_BIT)); + store[1] = static_cast(value >> (2U * CHAR_BIT)); + store[0] = static_cast(value >> (3U * CHAR_BIT)); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const_pointer store, T& value) { if (Endian == etl::endianness::value()) { @@ -476,7 +480,7 @@ namespace etl } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int endian_src, char* dst) + static ETL_CONSTEXPR14 void copy(const_pointer src, int endian_src, unsigned char* dst) { if (Endian == endian_src) { @@ -501,34 +505,34 @@ namespace etl template struct unaligned_copy { - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { if (Endian == etl::endianness::value()) { - store[0] = static_cast(value); - store[1] = static_cast(value >> (1U * CHAR_BIT)); - store[2] = static_cast(value >> (2U * CHAR_BIT)); - store[3] = static_cast(value >> (3U * CHAR_BIT)); - store[4] = static_cast(value >> (4U * CHAR_BIT)); - store[5] = static_cast(value >> (5U * CHAR_BIT)); - store[6] = static_cast(value >> (6U * CHAR_BIT)); - store[7] = static_cast(value >> (7U * CHAR_BIT)); + store[0] = static_cast(value); + store[1] = static_cast(value >> (1U * CHAR_BIT)); + store[2] = static_cast(value >> (2U * CHAR_BIT)); + store[3] = static_cast(value >> (3U * CHAR_BIT)); + store[4] = static_cast(value >> (4U * CHAR_BIT)); + store[5] = static_cast(value >> (5U * CHAR_BIT)); + store[6] = static_cast(value >> (6U * CHAR_BIT)); + store[7] = static_cast(value >> (7U * CHAR_BIT)); } else { - store[7] = static_cast(value); - store[6] = static_cast(value >> (1U * CHAR_BIT)); - store[5] = static_cast(value >> (2U * CHAR_BIT)); - store[4] = static_cast(value >> (3U * CHAR_BIT)); - store[3] = static_cast(value >> (4U * CHAR_BIT)); - store[2] = static_cast(value >> (5U * CHAR_BIT)); - store[1] = static_cast(value >> (6U * CHAR_BIT)); - store[0] = static_cast(value >> (7U * CHAR_BIT)); + store[7] = static_cast(value); + store[6] = static_cast(value >> (1U * CHAR_BIT)); + store[5] = static_cast(value >> (2U * CHAR_BIT)); + store[4] = static_cast(value >> (3U * CHAR_BIT)); + store[3] = static_cast(value >> (4U * CHAR_BIT)); + store[2] = static_cast(value >> (5U * CHAR_BIT)); + store[1] = static_cast(value >> (6U * CHAR_BIT)); + store[0] = static_cast(value >> (7U * CHAR_BIT)); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const_pointer store, T& value) { if (Endian == etl::endianness::value()) { @@ -555,7 +559,7 @@ namespace etl } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int endian_src, char* dst) + static ETL_CONSTEXPR14 void copy(const_pointer src, int endian_src, unsigned char* dst) { if (Endian == endian_src) { From 34cbc538fc78df1d57f8e7b56611c63660ea9ab4 Mon Sep 17 00:00:00 2001 From: benedekkupper Date: Mon, 3 Oct 2022 17:43:23 +0200 Subject: [PATCH 2/3] unaligned types: use unsigned storage type (#614) This saves one static_cast when copying from storage to value, and prevents unintended sign-extension when extracting raw data. --- include/etl/unaligned_type.h | 148 +++++++++++++++++------------------ 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/include/etl/unaligned_type.h b/include/etl/unaligned_type.h index 54b72205..0db9543a 100644 --- a/include/etl/unaligned_type.h +++ b/include/etl/unaligned_type.h @@ -58,10 +58,10 @@ namespace etl static ETL_CONSTANT size_t Size = Size_; - typedef char* pointer; - typedef const char* const_pointer; - typedef char* iterator; - typedef const char* const_iterator; + typedef unsigned char* pointer; + typedef const unsigned char* const_pointer; + typedef unsigned char* iterator; + typedef const unsigned char* const_iterator; typedef etl::reverse_iterator reverse_iterator; typedef etl::reverse_iterator const_reverse_iterator; @@ -196,7 +196,7 @@ namespace etl //************************************************************************* /// Index operator. //************************************************************************* - char& operator[](int i) + unsigned char& operator[](int i) { return storage[i]; } @@ -204,14 +204,14 @@ namespace etl //************************************************************************* /// Const index operator. //************************************************************************* - ETL_CONSTEXPR const char& operator[](int i) const + ETL_CONSTEXPR const unsigned char& operator[](int i) const { return storage[i]; } protected: - char storage[Size]; + unsigned char storage[Size]; }; } @@ -362,19 +362,19 @@ namespace etl struct unaligned_copy { //******************************* - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { store[0] = static_cast(value); } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const unsigned char* store, T& value) { value = static_cast(store[0]); } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int /*endian_src*/, char* dst) + static ETL_CONSTEXPR14 void copy(const unsigned char* src, int /*endian_src*/, unsigned char* dst) { dst[0] = src[0]; } @@ -387,37 +387,37 @@ namespace etl struct unaligned_copy { //******************************* - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { if (Endian == etl::endianness::value()) { - store[0] = static_cast(value); - store[1] = static_cast(value >> (1U * CHAR_BIT)); + store[0] = static_cast(value); + store[1] = static_cast(value >> (1U * CHAR_BIT)); } else { - store[1] = static_cast(value); - store[0] = static_cast(value >> (1U * CHAR_BIT)); + store[1] = static_cast(value); + store[0] = static_cast(value >> (1U * CHAR_BIT)); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const unsigned char* store, T& value) { if (Endian == etl::endianness::value()) { - value = static_cast(static_cast(store[0])); - value |= static_cast(static_cast(store[1])) << (1U * CHAR_BIT); + value = static_cast(store[0]); + value |= static_cast(store[1]) << (1U * CHAR_BIT); } else { - value = static_cast(static_cast(store[1])); - value |= static_cast(static_cast(store[0])) << (1U * CHAR_BIT); + value = static_cast(store[1]); + value |= static_cast(store[0]) << (1U * CHAR_BIT); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int endian_src, char* dst) + static ETL_CONSTEXPR14 void copy(const unsigned char* src, int endian_src, unsigned char* dst) { if (Endian == endian_src) { @@ -438,21 +438,21 @@ namespace etl template struct unaligned_copy { - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { if (Endian == etl::endianness::value()) { - store[0] = static_cast(value); - store[1] = static_cast(value >> (1U * CHAR_BIT)); - store[2] = static_cast(value >> (2U * CHAR_BIT)); - store[3] = static_cast(value >> (3U * CHAR_BIT)); + store[0] = static_cast(value); + store[1] = static_cast(value >> (1U * CHAR_BIT)); + store[2] = static_cast(value >> (2U * CHAR_BIT)); + store[3] = static_cast(value >> (3U * CHAR_BIT)); } else { - store[3] = static_cast(value); - store[2] = static_cast(value >> (1U * CHAR_BIT)); - store[1] = static_cast(value >> (2U * CHAR_BIT)); - store[0] = static_cast(value >> (3U * CHAR_BIT)); + store[3] = static_cast(value); + store[2] = static_cast(value >> (1U * CHAR_BIT)); + store[1] = static_cast(value >> (2U * CHAR_BIT)); + store[0] = static_cast(value >> (3U * CHAR_BIT)); } } @@ -461,22 +461,22 @@ namespace etl { if (Endian == etl::endianness::value()) { - value = static_cast(static_cast(store[0])); - value |= static_cast(static_cast(store[1])) << (1U * CHAR_BIT); - value |= static_cast(static_cast(store[2])) << (2U * CHAR_BIT); - value |= static_cast(static_cast(store[3])) << (3U * CHAR_BIT); + value = static_cast(store[0]); + value |= static_cast(store[1]) << (1U * CHAR_BIT); + value |= static_cast(store[2]) << (2U * CHAR_BIT); + value |= static_cast(store[3]) << (3U * CHAR_BIT); } else { - value = static_cast(static_cast(store[3])); - value |= static_cast(static_cast(store[2])) << (1U * CHAR_BIT); - value |= static_cast(static_cast(store[1])) << (2U * CHAR_BIT); - value |= static_cast(static_cast(store[0])) << (3U * CHAR_BIT); + value = static_cast(store[3]); + value |= static_cast(store[2]) << (1U * CHAR_BIT); + value |= static_cast(store[1]) << (2U * CHAR_BIT); + value |= static_cast(store[0]) << (3U * CHAR_BIT); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int endian_src, char* dst) + static ETL_CONSTEXPR14 void copy(const unsigned char* src, int endian_src, unsigned char* dst) { if (Endian == endian_src) { @@ -501,61 +501,61 @@ namespace etl template struct unaligned_copy { - static ETL_CONSTEXPR14 void copy(T value, char* store) + static ETL_CONSTEXPR14 void copy(T value, unsigned char* store) { if (Endian == etl::endianness::value()) { - store[0] = static_cast(value); - store[1] = static_cast(value >> (1U * CHAR_BIT)); - store[2] = static_cast(value >> (2U * CHAR_BIT)); - store[3] = static_cast(value >> (3U * CHAR_BIT)); - store[4] = static_cast(value >> (4U * CHAR_BIT)); - store[5] = static_cast(value >> (5U * CHAR_BIT)); - store[6] = static_cast(value >> (6U * CHAR_BIT)); - store[7] = static_cast(value >> (7U * CHAR_BIT)); + store[0] = static_cast(value); + store[1] = static_cast(value >> (1U * CHAR_BIT)); + store[2] = static_cast(value >> (2U * CHAR_BIT)); + store[3] = static_cast(value >> (3U * CHAR_BIT)); + store[4] = static_cast(value >> (4U * CHAR_BIT)); + store[5] = static_cast(value >> (5U * CHAR_BIT)); + store[6] = static_cast(value >> (6U * CHAR_BIT)); + store[7] = static_cast(value >> (7U * CHAR_BIT)); } else { - store[7] = static_cast(value); - store[6] = static_cast(value >> (1U * CHAR_BIT)); - store[5] = static_cast(value >> (2U * CHAR_BIT)); - store[4] = static_cast(value >> (3U * CHAR_BIT)); - store[3] = static_cast(value >> (4U * CHAR_BIT)); - store[2] = static_cast(value >> (5U * CHAR_BIT)); - store[1] = static_cast(value >> (6U * CHAR_BIT)); - store[0] = static_cast(value >> (7U * CHAR_BIT)); + store[7] = static_cast(value); + store[6] = static_cast(value >> (1U * CHAR_BIT)); + store[5] = static_cast(value >> (2U * CHAR_BIT)); + store[4] = static_cast(value >> (3U * CHAR_BIT)); + store[3] = static_cast(value >> (4U * CHAR_BIT)); + store[2] = static_cast(value >> (5U * CHAR_BIT)); + store[1] = static_cast(value >> (6U * CHAR_BIT)); + store[0] = static_cast(value >> (7U * CHAR_BIT)); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* store, T& value) + static ETL_CONSTEXPR14 void copy(const unsigned char* store, T& value) { if (Endian == etl::endianness::value()) { - value = static_cast(static_cast(store[0])); - value |= static_cast(static_cast(store[1])) << (1U * CHAR_BIT); - value |= static_cast(static_cast(store[2])) << (2U * CHAR_BIT); - value |= static_cast(static_cast(store[3])) << (3U * CHAR_BIT); - value |= static_cast(static_cast(store[4])) << (4U * CHAR_BIT); - value |= static_cast(static_cast(store[5])) << (5U * CHAR_BIT); - value |= static_cast(static_cast(store[6])) << (6U * CHAR_BIT); - value |= static_cast(static_cast(store[7])) << (7U * CHAR_BIT); + value = static_cast(store[0]); + value |= static_cast(store[1]) << (1U * CHAR_BIT); + value |= static_cast(store[2]) << (2U * CHAR_BIT); + value |= static_cast(store[3]) << (3U * CHAR_BIT); + value |= static_cast(store[4]) << (4U * CHAR_BIT); + value |= static_cast(store[5]) << (5U * CHAR_BIT); + value |= static_cast(store[6]) << (6U * CHAR_BIT); + value |= static_cast(store[7]) << (7U * CHAR_BIT); } else { - value = static_cast(static_cast(store[7])); - value |= static_cast(static_cast(store[6])) << (1U * CHAR_BIT); - value |= static_cast(static_cast(store[5])) << (2U * CHAR_BIT); - value |= static_cast(static_cast(store[4])) << (3U * CHAR_BIT); - value |= static_cast(static_cast(store[3])) << (4U * CHAR_BIT); - value |= static_cast(static_cast(store[2])) << (5U * CHAR_BIT); - value |= static_cast(static_cast(store[1])) << (6U * CHAR_BIT); - value |= static_cast(static_cast(store[0])) << (7U * CHAR_BIT); + value = static_cast(store[7]); + value |= static_cast(store[6]) << (1U * CHAR_BIT); + value |= static_cast(store[5]) << (2U * CHAR_BIT); + value |= static_cast(store[4]) << (3U * CHAR_BIT); + value |= static_cast(store[3]) << (4U * CHAR_BIT); + value |= static_cast(store[2]) << (5U * CHAR_BIT); + value |= static_cast(store[1]) << (6U * CHAR_BIT); + value |= static_cast(store[0]) << (7U * CHAR_BIT); } } //******************************* - static ETL_CONSTEXPR14 void copy(const char* src, int endian_src, char* dst) + static ETL_CONSTEXPR14 void copy(const unsigned char* src, int endian_src, unsigned char* dst) { if (Endian == endian_src) { From 644c95001606cc5d8a6f57028fc436c8b6e7df54 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Mon, 3 Oct 2022 16:46:05 +0100 Subject: [PATCH 3/3] Storage type to unsigned char --- include/etl/unaligned_type.h | 21 +++++++++++++++------ test/test_unaligned_type.cpp | 18 ++++++++++++++++++ test/test_unaligned_type_constexpr.cpp | 22 ++++++++++++++++++++-- 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/include/etl/unaligned_type.h b/include/etl/unaligned_type.h index 8a65f54c..099858a2 100644 --- a/include/etl/unaligned_type.h +++ b/include/etl/unaligned_type.h @@ -228,8 +228,13 @@ namespace etl typedef T value_type; - typedef typename private_unaligned_type::unaligned_type_common::pointer pointer; - typedef typename private_unaligned_type::unaligned_type_common::const_pointer const_pointer; + typedef typename private_unaligned_type::unaligned_type_common::storage_type storage_type; + typedef typename private_unaligned_type::unaligned_type_common::pointer pointer; + typedef typename private_unaligned_type::unaligned_type_common::const_pointer const_pointer; + typedef typename private_unaligned_type::unaligned_type_common::iterator iterator; + typedef typename private_unaligned_type::unaligned_type_common::const_iterator const_iterator; + typedef typename private_unaligned_type::unaligned_type_common::reverse_iterator reverse_iterator; + typedef typename private_unaligned_type::unaligned_type_common::const_reverse_iterator const_reverse_iterator; static ETL_CONSTANT int Endian = Endian_; static ETL_CONSTANT size_t Size = private_unaligned_type::unaligned_type_common::Size; @@ -360,7 +365,8 @@ namespace etl struct unaligned_copy; //******************************************* - // Size == 1 + /// Unaligned copy + /// Size == 1 //******************************************* template struct unaligned_copy @@ -385,7 +391,8 @@ namespace etl }; //******************************************* - // Size == 2 + /// Unaligned copy + /// Size == 2 //******************************************* template struct unaligned_copy @@ -437,7 +444,8 @@ namespace etl }; //******************************************* - // Size == 4 + /// Unaligned copy + /// Size == 4 //******************************************* template struct unaligned_copy @@ -500,7 +508,8 @@ namespace etl }; //******************************************* - // Size == 8 + /// Unaligned copy + /// Size == 8 //******************************************* template struct unaligned_copy diff --git a/test/test_unaligned_type.cpp b/test/test_unaligned_type.cpp index 0bfe70d0..26b41697 100644 --- a/test/test_unaligned_type.cpp +++ b/test/test_unaligned_type.cpp @@ -760,5 +760,23 @@ namespace CHECK_EQUAL(0x1234, test_le.value()); CHECK_EQUAL(0x1234, test_be.value()); } + + //************************************************************************* + TEST(test_storage_bytes) + { + etl::le_uint16_t test_le(0x1234); + etl::be_uint16_t test_be(0x1234); + + int lev0 = test_le[0]; + int lev1 = test_le[1]; + + int bev0 = test_be[0]; + int bev1 = test_be[1]; + + CHECK_EQUAL(0x34, lev0); + CHECK_EQUAL(0x12, lev1); + CHECK_EQUAL(0x12, bev0); + CHECK_EQUAL(0x34, bev1); + } }; } diff --git a/test/test_unaligned_type_constexpr.cpp b/test/test_unaligned_type_constexpr.cpp index e77ad269..ddf65fa1 100644 --- a/test/test_unaligned_type_constexpr.cpp +++ b/test/test_unaligned_type_constexpr.cpp @@ -544,8 +544,8 @@ namespace { static constexpr etl::be_uint16_t test(0x1234); - constexpr const char* p1 = test.data(); - constexpr const char* p2 = test.data() + 1U; + constexpr etl::be_uint16_t::const_pointer p1 = test.data(); + constexpr etl::be_uint16_t::const_pointer p2 = test.data() + 1U; CHECK_EQUAL(0x12, *p1); CHECK_EQUAL(0x34, *p2); @@ -618,5 +618,23 @@ namespace CHECK_EQUAL(0x1234, lev); CHECK_EQUAL(0x1234, bev); } + + //************************************************************************* + TEST(test_storage_bytes) + { + constexpr etl::le_uint16_t test_le(0x1234); + constexpr etl::be_uint16_t test_be(0x1234); + + constexpr int lev0 = test_le[0]; + constexpr int lev1 = test_le[1]; + + constexpr int bev0 = test_be[0]; + constexpr int bev1 = test_be[1]; + + CHECK_EQUAL(0x34, lev0); + CHECK_EQUAL(0x12, lev1); + CHECK_EQUAL(0x12, bev0); + CHECK_EQUAL(0x34, bev1); + } }; }