diff --git a/src/lib/core/CHIPEncoding.h b/src/lib/core/CHIPEncoding.h index c1a6783fa1099a..09079b7543b3a9 100644 --- a/src/lib/core/CHIPEncoding.h +++ b/src/lib/core/CHIPEncoding.h @@ -216,6 +216,13 @@ namespace LittleEndian { template inline T HostSwap(T v); +// For completeness of the set, we have identity. +template <> +inline uint8_t HostSwap(uint8_t v) +{ + return v; +} + /** * This conditionally performs, as necessary for the target system, a * byte order swap by value of the specified 16-bit value, presumed to diff --git a/src/lib/support/BufferReader.cpp b/src/lib/support/BufferReader.cpp index 3e9ede6d006710..0d214774e41efd 100644 --- a/src/lib/support/BufferReader.cpp +++ b/src/lib/support/BufferReader.cpp @@ -17,40 +17,46 @@ #include "BufferReader.h" +#include + #include +#include namespace chip { namespace Encoding { namespace LittleEndian { namespace { -// These helper methods return void and put the value being read into an + +// This helper methods return void and put the value being read into an // outparam because that allows us to easily overload on the type of the // thing being read. -void ReadHelper(const uint8_t *& p, uint8_t * dest) -{ - *dest = Read8(p); -} -void ReadHelper(const uint8_t *& p, uint16_t * dest) -{ - *dest = Read16(p); -} -void ReadHelper(const uint8_t *& p, uint32_t * dest) +void ReadHelper(const uint8_t * p, bool * dest) { - *dest = Read32(p); + *dest = (*p != 0); } -void ReadHelper(const uint8_t *& p, uint64_t * dest) + +template +void ReadHelper(const uint8_t * p, T * dest) { - *dest = Read64(p); + std::make_unsigned_t result; + memcpy(&result, p, sizeof(result)); + result = chip::Encoding::LittleEndian::HostSwap(result); + + *dest = static_cast(result); } + } // anonymous namespace template -void Reader::RawRead(T * retval) +void Reader::RawReadLowLevelBeCareful(T * retval) { static_assert(CHAR_BIT == 8, "Our various sizeof checks rely on bytes and octets being the same thing"); + static_assert((-1 & 3) == 3, "LittleEndian::BufferReader only works with 2's complement architectures."); + + VerifyOrReturn(IsSuccess()); - static constexpr size_t data_size = sizeof(T); + constexpr size_t data_size = sizeof(T); if (mAvailable < data_size) { @@ -61,6 +67,8 @@ void Reader::RawRead(T * retval) } ReadHelper(mReadPtr, retval); + mReadPtr += data_size; + mAvailable = static_cast(mAvailable - data_size); } @@ -84,10 +92,15 @@ Reader & Reader::ReadBytes(uint8_t * dest, size_t size) } // Explicit Read instantiations for the data types we want to support. -template void Reader::RawRead(uint8_t *); -template void Reader::RawRead(uint16_t *); -template void Reader::RawRead(uint32_t *); -template void Reader::RawRead(uint64_t *); +template void Reader::RawReadLowLevelBeCareful(bool *); +template void Reader::RawReadLowLevelBeCareful(int8_t *); +template void Reader::RawReadLowLevelBeCareful(int16_t *); +template void Reader::RawReadLowLevelBeCareful(int32_t *); +template void Reader::RawReadLowLevelBeCareful(int64_t *); +template void Reader::RawReadLowLevelBeCareful(uint8_t *); +template void Reader::RawReadLowLevelBeCareful(uint16_t *); +template void Reader::RawReadLowLevelBeCareful(uint32_t *); +template void Reader::RawReadLowLevelBeCareful(uint64_t *); } // namespace LittleEndian } // namespace Encoding diff --git a/src/lib/support/BufferReader.h b/src/lib/support/BufferReader.h index bbf21f529d8fb6..14c251bd33f94d 100644 --- a/src/lib/support/BufferReader.h +++ b/src/lib/support/BufferReader.h @@ -90,6 +90,28 @@ class Reader CHECK_RETURN_VALUE CHIP_ERROR StatusCode() const { return mStatus; } + /** + * @return false if the reader is in error, true if the reader is OK. + */ + bool IsSuccess() const { return StatusCode() == CHIP_NO_ERROR; } + + /** + * Read a bool, assuming single byte storage. + * + * @param [out] dest Where the 8-bit integer goes. + * + * @note The read can put the reader in a failed-status state if there are + * not enough octets available. Callers must either continue to do + * more reads on the return value or check its status to see whether + * the sequence of reads that has been performed succeeded. + */ + CHECK_RETURN_VALUE + Reader & ReadBool(bool * dest) + { + RawReadLowLevelBeCareful(dest); + return *this; + } + /** * Read a single 8-bit unsigned integer. * @@ -103,7 +125,7 @@ class Reader CHECK_RETURN_VALUE Reader & Read8(uint8_t * dest) { - RawRead(dest); + RawReadLowLevelBeCareful(dest); return *this; } @@ -120,7 +142,7 @@ class Reader CHECK_RETURN_VALUE Reader & Read16(uint16_t * dest) { - RawRead(dest); + RawReadLowLevelBeCareful(dest); return *this; } @@ -137,7 +159,7 @@ class Reader CHECK_RETURN_VALUE Reader & Read32(uint32_t * dest) { - RawRead(dest); + RawReadLowLevelBeCareful(dest); return *this; } @@ -154,7 +176,75 @@ class Reader CHECK_RETURN_VALUE Reader & Read64(uint64_t * dest) { - RawRead(dest); + RawReadLowLevelBeCareful(dest); + return *this; + } + + /** + * Read a single 8-bit signed integer. + * + * @param [out] dest Where the 8-bit integer goes. + * + * @note The read can put the reader in a failed-status state if there are + * not enough octets available. Callers must either continue to do + * more reads on the return value or check its status to see whether + * the sequence of reads that has been performed succeeded. + */ + CHECK_RETURN_VALUE + Reader & ReadSigned8(int8_t * dest) + { + RawReadLowLevelBeCareful(dest); + return *this; + } + + /** + * Read a single 16-bit signed integer. + * + * @param [out] dest Where the 16-bit integer goes. + * + * @note The read can put the reader in a failed-status state if there are + * not enough octets available. Callers must either continue to do + * more reads on the return value or check its status to see whether + * the sequence of reads that has been performed succeeded. + */ + CHECK_RETURN_VALUE + Reader & ReadSigned16(int16_t * dest) + { + RawReadLowLevelBeCareful(dest); + return *this; + } + + /** + * Read a single 32-bit signed integer. + * + * @param [out] dest Where the 32-bit integer goes. + * + * @note The read can put the reader in a failed-status state if there are + * not enough octets available. Callers must either continue to do + * more reads on the return value or check its status to see whether + * the sequence of reads that has been performed succeeded. + */ + CHECK_RETURN_VALUE + Reader & ReadSigned32(int32_t * dest) + { + RawReadLowLevelBeCareful(dest); + return *this; + } + + /** + * Read a single 64-bit signed integer. + * + * @param [out] dest Where the 64-bit integer goes. + * + * @note The read can put the reader in a failed-status state if there are + * not enough octets available. Callers must either continue to do + * more reads on the return value or check its status to see whether + * the sequence of reads that has been performed succeeded. + */ + CHECK_RETURN_VALUE + Reader & ReadSigned64(int64_t * dest) + { + RawReadLowLevelBeCareful(dest); return *this; } @@ -180,7 +270,7 @@ class Reader * delegate to this one. */ template - void RawRead(T * retval); + void RawReadLowLevelBeCareful(T * retval); /** * Advance the Reader forward by the specified number of octets. diff --git a/src/lib/support/BufferWriter.cpp b/src/lib/support/BufferWriter.cpp index e9394703cbea28..c606cba6e2b37d 100644 --- a/src/lib/support/BufferWriter.cpp +++ b/src/lib/support/BufferWriter.cpp @@ -60,6 +60,17 @@ LittleEndian::BufferWriter & LittleEndian::BufferWriter::EndianPut(uint64_t x, s return *this; } +LittleEndian::BufferWriter & LittleEndian::BufferWriter::EndianPutSigned(int64_t x, size_t size) +{ + while (size > 0) + { + Put(static_cast(x & 0xff)); + x >>= 8; + size--; + } + return *this; +} + BigEndian::BufferWriter & BigEndian::BufferWriter::EndianPut(uint64_t x, size_t size) { while (size-- > 0) @@ -69,5 +80,14 @@ BigEndian::BufferWriter & BigEndian::BufferWriter::EndianPut(uint64_t x, size_t return *this; } +BigEndian::BufferWriter & BigEndian::BufferWriter::EndianPutSigned(int64_t x, size_t size) +{ + while (size-- > 0) + { + Put(static_cast((x >> (size * 8)) & 0xff)); + } + return *this; +} + } // namespace Encoding } // namespace chip diff --git a/src/lib/support/BufferWriter.h b/src/lib/support/BufferWriter.h index a8afee2a7305b9..72f50b327ea369 100644 --- a/src/lib/support/BufferWriter.h +++ b/src/lib/support/BufferWriter.h @@ -104,13 +104,18 @@ class EndianBufferWriterBase : public BufferWriter Derived & Put(uint8_t c) { return static_cast(BufferWriter::Put(c)); } Derived & Skip(size_t len) { return static_cast(BufferWriter::Skip(len)); } - // write an integer into a buffer, in an endian specific way + // write an integer into a buffer, in an endian-specific way Derived & Put8(uint8_t c) { return static_cast(this)->Put(c); } Derived & Put16(uint16_t x) { return static_cast(this)->EndianPut(x, sizeof(x)); } Derived & Put32(uint32_t x) { return static_cast(this)->EndianPut(x, sizeof(x)); } Derived & Put64(uint64_t x) { return static_cast(this)->EndianPut(x, sizeof(x)); } + Derived & PutSigned8(int8_t x) { return static_cast(this)->EndianPutSigned(x, sizeof(x)); } + Derived & PutSigned16(int16_t x) { return static_cast(this)->EndianPutSigned(x, sizeof(x)); } + Derived & PutSigned32(int32_t x) { return static_cast(this)->EndianPutSigned(x, sizeof(x)); } + Derived & PutSigned64(int64_t x) { return static_cast(this)->EndianPutSigned(x, sizeof(x)); } + protected: EndianBufferWriterBase(uint8_t * buf, size_t len) : BufferWriter(buf, len) {} EndianBufferWriterBase(MutableByteSpan buf) : BufferWriter(buf.data(), buf.size()) {} @@ -123,11 +128,15 @@ namespace LittleEndian { class BufferWriter : public EndianBufferWriterBase { public: - BufferWriter(uint8_t * buf, size_t len) : EndianBufferWriterBase(buf, len) {} + BufferWriter(uint8_t * buf, size_t len) : EndianBufferWriterBase(buf, len) + { + static_assert((-1 & 3) == 3, "LittleEndian::BufferWriter only works with 2's complement architectures."); + } BufferWriter(MutableByteSpan buf) : EndianBufferWriterBase(buf) {} BufferWriter(const BufferWriter & other) = default; BufferWriter & operator=(const BufferWriter & other) = default; BufferWriter & EndianPut(uint64_t x, size_t size); + BufferWriter & EndianPutSigned(int64_t x, size_t size); }; } // namespace LittleEndian @@ -137,11 +146,15 @@ namespace BigEndian { class BufferWriter : public EndianBufferWriterBase { public: - BufferWriter(uint8_t * buf, size_t len) : EndianBufferWriterBase(buf, len) {} + BufferWriter(uint8_t * buf, size_t len) : EndianBufferWriterBase(buf, len) + { + static_assert((-1 & 3) == 3, "BigEndian::BufferWriter only works with 2's complement architectures."); + } BufferWriter(MutableByteSpan buf) : EndianBufferWriterBase(buf) {} BufferWriter(const BufferWriter & other) = default; BufferWriter & operator=(const BufferWriter & other) = default; BufferWriter & EndianPut(uint64_t x, size_t size); + BufferWriter & EndianPutSigned(int64_t x, size_t size); }; } // namespace BigEndian diff --git a/src/lib/support/tests/TestBufferReader.cpp b/src/lib/support/tests/TestBufferReader.cpp index ea501ee19ae47a..9af64c9c1b148f 100644 --- a/src/lib/support/tests/TestBufferReader.cpp +++ b/src/lib/support/tests/TestBufferReader.cpp @@ -133,12 +133,139 @@ static void TestBufferReader_Skip(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR); } +static void TestBufferReader_LittleEndianScalars(nlTestSuite * inSuite, void * inContext) +{ + const uint8_t test_buf1[10] = { 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 }; + + // Unsigned 8 bits reads + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + uint8_t val1 = 0; + uint8_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.Read8(&val1).Read8(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, val1 == 0xfe); + NL_TEST_ASSERT(inSuite, val2 == 0xff); + } + + // Unsigned 16 bits reads + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + uint16_t val1 = 0; + uint16_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.Read16(&val1).Read16(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, val1 == 0xfffe); + NL_TEST_ASSERT(inSuite, val2 == 0xffff); + } + + // Unsigned 32 bits reads + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + uint32_t val1 = 0; + uint32_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.Read32(&val1).Read32(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, val1 == static_cast(0xfffffffeUL)); + NL_TEST_ASSERT(inSuite, val2 == static_cast(0xffffffffUL)); + } + + // Unsigned 32 bits reads, unaligned + { + uint8_t test_buf2[10] = { 0x00, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 }; + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf2 } }; + + uint32_t val1 = 0; + uint32_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.Skip(1).Read32(&val1).Read32(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, reader.Remaining() == 1); + NL_TEST_ASSERT(inSuite, val1 == static_cast(0xfffffffeUL)); + NL_TEST_ASSERT(inSuite, val2 == static_cast(0xffffffffUL)); + } + + // Unsigned 64 bits read + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + uint64_t val = 0; + NL_TEST_ASSERT(inSuite, reader.Read64(&val).IsSuccess()); + NL_TEST_ASSERT(inSuite, reader.Remaining() == 2); + NL_TEST_ASSERT(inSuite, val == static_cast(0xfffffffffffffffeULL)); + } + + // Signed 8 bits reads + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + int8_t val1 = 0; + int8_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.ReadSigned8(&val1).ReadSigned8(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, val1 == -2); + NL_TEST_ASSERT(inSuite, val2 == -1); + } + + // Signed 16 bits reads + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + int16_t val1 = 0; + int16_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.ReadSigned16(&val1).ReadSigned16(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, val1 == -2); + NL_TEST_ASSERT(inSuite, val2 == -1); + } + + // Signed 32 bits reads + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + int32_t val1 = 0; + int32_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.ReadSigned32(&val1).ReadSigned32(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, val1 == -2); + NL_TEST_ASSERT(inSuite, val2 == -1); + } + + // Signed 32 bits reads, unaligned + { + uint8_t test_buf2[10] = { 0x00, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 }; + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf2 } }; + + int32_t val1 = 0; + int32_t val2 = 0; + NL_TEST_ASSERT(inSuite, reader.Skip(1).ReadSigned32(&val1).ReadSigned32(&val2).IsSuccess()); + NL_TEST_ASSERT(inSuite, reader.Remaining() == 1); + NL_TEST_ASSERT(inSuite, val1 == static_cast(-2L)); + NL_TEST_ASSERT(inSuite, val2 == static_cast(-1L)); + } + + // Signed 64 bits read + { + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf1 } }; + int64_t val = 0; + NL_TEST_ASSERT(inSuite, reader.ReadSigned64(&val).IsSuccess()); + NL_TEST_ASSERT(inSuite, reader.Remaining() == 2); + NL_TEST_ASSERT(inSuite, val == static_cast(-2LL)); + } + + // Bools + { + uint8_t test_buf2[5] = { 0x00, 0xff, 0x01, 0x04, 0x07 }; + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf2 } }; + bool val1 = true; + bool val2 = false; + bool val3 = false; + + NL_TEST_ASSERT(inSuite, reader.ReadBool(&val1).ReadBool(&val2).ReadBool(&val3).IsSuccess()); + NL_TEST_ASSERT(inSuite, reader.Remaining() == 2); + NL_TEST_ASSERT(inSuite, val1 == false); + NL_TEST_ASSERT(inSuite, val2 == true); + NL_TEST_ASSERT(inSuite, val3 == true); + } +} + #define NL_TEST_DEF_FN(fn) NL_TEST_DEF("Test " #fn, fn) /** * Test Suite. It lists all the test functions. */ -static const nlTest sTests[] = { NL_TEST_DEF_FN(TestBufferReader_Basic), NL_TEST_DEF_FN(TestBufferReader_BasicSpan), - NL_TEST_DEF_FN(TestBufferReader_Saturation), NL_TEST_DEF_FN(TestBufferReader_Skip), +static const nlTest sTests[] = { NL_TEST_DEF_FN(TestBufferReader_Basic), + NL_TEST_DEF_FN(TestBufferReader_BasicSpan), + NL_TEST_DEF_FN(TestBufferReader_Saturation), + NL_TEST_DEF_FN(TestBufferReader_Skip), + NL_TEST_DEF("Test Little-endian buffer Reader scalar reads", TestBufferReader_LittleEndianScalars), NL_TEST_SENTINEL() }; int TestBufferReader() diff --git a/src/lib/support/tests/TestBufferWriter.cpp b/src/lib/support/tests/TestBufferWriter.cpp index 53a5012172f675..2e8d187ae6a15d 100644 --- a/src/lib/support/tests/TestBufferWriter.cpp +++ b/src/lib/support/tests/TestBufferWriter.cpp @@ -196,6 +196,42 @@ void TestPutLittleEndian(nlTestSuite * inSuite, void * inContext) bb.EndianPut(0x0102030405060708u, 3); NL_TEST_ASSERT(inSuite, bb.expect("\x08\x07\x06", 3, 0)); } + + { + BWTest bb(4); + bb.PutSigned8(static_cast(-6)); + NL_TEST_ASSERT(inSuite, bb.expect("\xfa", 1, 3)); + } + + { + BWTest bb(4); + bb.PutSigned16(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xfe\xff", 2, 2)); + } + + { + BWTest bb(4); + bb.PutSigned32(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xfe\xff\xff\xff", 4, 0)); + } + + { + BWTest bb(8); + bb.PutSigned64(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xfe\xff\xff\xff\xff\xff\xff\xff", 8, 0)); + } + + { + BWTest bb(7); + bb.PutSigned64(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xfe\xff\xff\xff\xff\xff\xff", 8, 0)); + } + + { + BWTest bb(9); + bb.PutSigned64(static_cast(9223372036854775807LL)); + NL_TEST_ASSERT(inSuite, bb.expect("\xff\xff\xff\xff\xff\xff\xff\x7f", 8, 1)); + } } void TestPutBigEndian(nlTestSuite * inSuite, void * inContext) @@ -223,6 +259,42 @@ void TestPutBigEndian(nlTestSuite * inSuite, void * inContext) bb.EndianPut(0x0102030405060708u, 3); NL_TEST_ASSERT(inSuite, bb.expect("\x06\x07\x08", 3, 0)); } + + { + BWTest bb(4); + bb.PutSigned8(static_cast(-6)); + NL_TEST_ASSERT(inSuite, bb.expect("\xfa", 1, 3)); + } + + { + BWTest bb(4); + bb.PutSigned16(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xff\xfe", 2, 2)); + } + + { + BWTest bb(4); + bb.PutSigned32(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xff\xff\xff\xfe", 4, 0)); + } + + { + BWTest bb(8); + bb.PutSigned64(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xff\xff\xff\xff\xff\xff\xff\xfe", 8, 0)); + } + + { + BWTest bb(7); + bb.PutSigned64(static_cast(-2)); + NL_TEST_ASSERT(inSuite, bb.expect("\xff\xff\xff\xff\xff\xff\xff", 8, 0)); + } + + { + BWTest bb(9); + bb.PutSigned64(static_cast(9223372036854775807LL)); + NL_TEST_ASSERT(inSuite, bb.expect("\x7f\xff\xff\xff\xff\xff\xff\xff", 8, 1)); + } } const nlTest sTests[] = {