Skip to content

Commit

Permalink
TLV reader: Improve floating-point support
Browse files Browse the repository at this point in the history
1. For some reason the Get(float&) signature was declared in the .h file
   but was never defined. Define it as well, extracting a helper
   function to share logic between Get(float&) and Get(double&).
2. It is undefined behaviour to use unions for type-punning in C++. Use
   raw memcpy calls instead, which is defined by the C++ standard
   (std::bit_cast is not available).

   Compilers will generally be able to optimize this away unless a
   freestanding implementation (with possibly non-standard memcpy) is
   used.
  • Loading branch information
anqid-g committed Jul 21, 2021
1 parent af1cc26 commit 7b07d8f
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 20 deletions.
42 changes: 26 additions & 16 deletions src/lib/core/CHIPTLVReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,35 +207,45 @@ CHIP_ERROR TLVReader::Get(uint64_t & v)
return CHIP_NO_ERROR;
}

CHIP_ERROR TLVReader::Get(double & v)
namespace {
template <typename T>
CHIP_ERROR GetFloatingImpl(const TLVElementType elementType, const uint64_t elemLenOrVal, T & v)
{
switch (ElementType())
switch (elementType)
{
case TLVElementType::FloatingPointNumber32: {
union
{
uint32_t u32;
float f;
} cvt;
cvt.u32 = static_cast<uint32_t>(mElemLenOrVal);
v = cvt.f;
float f;
auto u32 = static_cast<uint32_t>(elemLenOrVal);
memcpy(&f, &u32, sizeof(f));
v = f;
break;
}
case TLVElementType::FloatingPointNumber64: {
union
{
uint64_t u64;
double d;
} cvt;
cvt.u64 = mElemLenOrVal;
v = cvt.d;
double d;
memcpy(&d, &elemLenOrVal, sizeof(d));
v = static_cast<T>(d);
break;
}
default:
return CHIP_ERROR_WRONG_TLV_TYPE;
}
return CHIP_NO_ERROR;
}
} // namespace

// Note: Unlike the integer Get functions, this code avoids doing conversions
// between float and double wherever possible, because these conversions are
// relatively expensive on platforms that use soft-float instruction sets.

CHIP_ERROR TLVReader::Get(float & v)
{
return GetFloatingImpl(ElementType(), mElemLenOrVal, v);
}

CHIP_ERROR TLVReader::Get(double & v)
{
return GetFloatingImpl(ElementType(), mElemLenOrVal, v);
}

CHIP_ERROR TLVReader::GetBytes(uint8_t * buf, uint32_t bufSize)
{
Expand Down
15 changes: 11 additions & 4 deletions src/lib/core/tests/TestCHIPTLV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,12 @@ void ReadEncoding1(nlTestSuite * inSuite, TLVReader & reader)

TestNext<TLVReader>(inSuite, reader2);

TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535),
static_cast<float>(17.9));
TestGet<TLVReader, float>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);
TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);

TestNext<TLVReader>(inSuite, reader2);

TestGet<TLVReader, float>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65536), 17.9f);
TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65536), 17.9);

TestEndAndCloseContainer(inSuite, reader, reader2);
Expand Down Expand Up @@ -2966,11 +2967,12 @@ void TestCHIPTLVReaderDup(nlTestSuite * inSuite)

TestNext<TLVReader>(inSuite, reader2);

TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535),
static_cast<float>(17.9));
TestGet<TLVReader, float>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);
TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65535), 17.9f);

TestNext<TLVReader>(inSuite, reader2);

TestGet<TLVReader, float>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65536), 17.9f);
TestGet<TLVReader, double>(inSuite, reader2, kTLVType_FloatingPointNumber, ProfileTag(TestProfile_2, 65536), 17.9);

TestEndAndCloseContainer(inSuite, reader, reader2);
Expand All @@ -2995,6 +2997,11 @@ void TestCHIPTLVReaderErrorHandling(nlTestSuite * inSuite)
err = reader.Get(val);
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_WRONG_TLV_TYPE);

// Get(float&)
float numF;
err = reader.Get(numF);
NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_WRONG_TLV_TYPE);

// Get(double&)
double numD;
err = reader.Get(numD);
Expand Down

0 comments on commit 7b07d8f

Please sign in to comment.