Skip to content

Commit

Permalink
Simplify our setup for serializing integer sets. (#6800)
Browse files Browse the repository at this point in the history 
We can store binary data now, so don't need to jump through the Base64
hoops.  We do still want to preserve the "serialized as known-endian"
property, though.
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple authored May 14, 2021
1 parent 6d42abd commit f72e0b7
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 101 deletions.
41 changes: 14 additions & 27 deletions src/controller/CHIPDeviceController.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,15 +582,15 @@ uint16_t DeviceController::FindDeviceIndex(NodeId id)

CHIP_ERROR DeviceController::InitializePairedDeviceList()
{
CHIP_ERROR err = CHIP_NO_ERROR;
char * buffer = nullptr;
CHIP_ERROR err = CHIP_NO_ERROR;
uint8_t * buffer = nullptr;

VerifyOrExit(mStorageDelegate != nullptr, err = CHIP_ERROR_INCORRECT_STATE);

if (!mPairedDevicesInitialized)
{
constexpr uint16_t max_size = CHIP_MAX_SERIALIZED_SIZE_U64(kNumMaxPairedDevices);
buffer = static_cast<char *>(chip::Platform::MemoryCalloc(max_size, 1));
constexpr uint16_t max_size = sizeof(uint64_t) * kNumMaxPairedDevices;
buffer = static_cast<uint8_t *>(chip::Platform::MemoryAlloc(max_size));
uint16_t size = max_size;

VerifyOrExit(buffer != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
Expand All @@ -603,7 +603,7 @@ CHIP_ERROR DeviceController::InitializePairedDeviceList()
if (lookupError != CHIP_ERROR_KEY_NOT_FOUND)
{
VerifyOrExit(size <= max_size, err = CHIP_ERROR_INVALID_DEVICE_DESCRIPTOR);
err = SetPairedDeviceList(buffer);
err = SetPairedDeviceList(ByteSpan(buffer, size));
SuccessOrExit(err);
}
}
Expand All @@ -621,15 +621,10 @@ CHIP_ERROR DeviceController::InitializePairedDeviceList()
return err;
}

CHIP_ERROR DeviceController::SetPairedDeviceList(const char * serialized)
CHIP_ERROR DeviceController::SetPairedDeviceList(ByteSpan serialized)
{
CHIP_ERROR err = CHIP_NO_ERROR;
size_t len = strlen(serialized) + 1;
uint16_t lenU16 = static_cast<uint16_t>(len);
VerifyOrExit(CanCastTo<uint16_t>(len), err = CHIP_ERROR_INVALID_ARGUMENT);
err = mPairedDevices.DeserializeBase64(serialized, lenU16);
CHIP_ERROR err = mPairedDevices.Deserialize(serialized);

exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed to recreate the device list with buffer %s\n", serialized);
Expand Down Expand Up @@ -1316,21 +1311,13 @@ void DeviceCommissioner::PersistDeviceList()
{
if (mStorageDelegate != nullptr && mPairedDevicesUpdated && mState == State::Initialized)
{
constexpr uint16_t size = CHIP_MAX_SERIALIZED_SIZE_U64(kNumMaxPairedDevices);
char * serialized = static_cast<char *>(chip::Platform::MemoryAlloc(size));
uint16_t requiredSize = size;
if (serialized != nullptr)
{
const char * value = mPairedDevices.SerializeBase64(serialized, requiredSize);
if (value != nullptr && requiredSize <= size)
{
// TODO: no need to base64 again the value
PERSISTENT_KEY_OP(static_cast<uint64_t>(0), kPairedDeviceListKeyPrefix, key,
mStorageDelegate->SyncSetKeyValue(key, value, requiredSize));
mPairedDevicesUpdated = false;
}
chip::Platform::MemoryFree(serialized);
}
mPairedDevices.Serialize([&](ByteSpan data) -> CHIP_ERROR {
VerifyOrReturnError(data.size() <= UINT16_MAX, CHIP_ERROR_INVALID_ARGUMENT);
PERSISTENT_KEY_OP(static_cast<uint64_t>(0), kPairedDeviceListKeyPrefix, key,
mStorageDelegate->SyncSetKeyValue(key, data.data(), static_cast<uint16_t>(data.size())));
mPairedDevicesUpdated = false;
return CHIP_NO_ERROR;
});
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceController.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <core/CHIPCore.h>
#include <core/CHIPPersistentStorageDelegate.h>
#include <core/CHIPTLV.h>
#include <lib/support/Span.h>
#include <messaging/ExchangeMgr.h>
#include <messaging/ExchangeMgrDelegate.h>
#include <protocols/secure_channel/MessageCounterManager.h>
Expand Down Expand Up @@ -271,7 +272,7 @@ class DLL_EXPORT DeviceController : public Messaging::ExchangeDelegate,
void ReleaseDevice(uint16_t index);
void ReleaseDeviceById(NodeId remoteDeviceId);
CHIP_ERROR InitializePairedDeviceList();
CHIP_ERROR SetPairedDeviceList(const char * pairedDeviceSerializedSet);
CHIP_ERROR SetPairedDeviceList(ByteSpan pairedDeviceSerializedSet);
ControllerDeviceInitParams GetControllerDeviceInitParams();

Transport::AdminId mAdminId = 0;
Expand Down
42 changes: 7 additions & 35 deletions src/lib/support/SerializableIntegerSet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,47 +19,19 @@
#include "SerializableIntegerSet.h"

#include <core/CHIPEncoding.h>
#include <lib/support/CodeUtils.h>

namespace chip {

const char * SerializableU64SetBase::SerializeBase64(char * buf, uint16_t & buflen)
CHIP_ERROR SerializableU64SetBase::Deserialize(ByteSpan serialized)
{
char * out = nullptr;
size_t len = sizeof(uint64_t) * mNextAvailable;
VerifyOrExit(buflen >= SerializedSize(), buflen = SerializedSize());
VerifyOrExit(buf != nullptr, buflen = SerializedSize());
VerifyOrReturnError(serialized.size() <= MaxSerializedSize(), CHIP_ERROR_INVALID_ARGUMENT);
memcpy(mData, serialized.data(), serialized.size());
mNextAvailable = static_cast<uint16_t>(serialized.size()) / static_cast<uint16_t>(sizeof(uint64_t));

// Swap to little endian order if needed.
// Our serialized data is always little-endian; swap to native.
SwapByteOrderIfNeeded();

buflen = Base64Encode(reinterpret_cast<const uint8_t *>(mData), static_cast<uint16_t>(len), buf);
buf[buflen] = '\0';
out = buf;

// Swap back to the original order
SwapByteOrderIfNeeded();

exit:
return out;
}

CHIP_ERROR SerializableU64SetBase::DeserializeBase64(const char * serialized, uint16_t buflen)
{
CHIP_ERROR err = CHIP_NO_ERROR;
uint16_t decodelen = 0;
VerifyOrExit(buflen <= MaxSerializedSize(), err = CHIP_ERROR_INVALID_ARGUMENT);

decodelen = Base64Decode(serialized, buflen, reinterpret_cast<uint8_t *>(mData));
VerifyOrExit(decodelen <= sizeof(uint64_t) * mCapacity, err = CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrExit(decodelen % sizeof(uint64_t) == 0, err = CHIP_ERROR_INVALID_ARGUMENT);

mNextAvailable = decodelen / static_cast<uint16_t>(sizeof(uint64_t));

// Swap from little endian if needed
SwapByteOrderIfNeeded();

exit:
return err;
return CHIP_NO_ERROR;
}

void SerializableU64SetBase::SwapByteOrderIfNeeded()
Expand Down
42 changes: 24 additions & 18 deletions src/lib/support/SerializableIntegerSet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,8 @@

#pragma once

#include <support/Base64.h>
#include <support/CodeUtils.h>

// BASE64_ENCODED_LEN doesn't account for null termination of the string.
// So, we are adding 1 extra byte to the size requirement.
#define CHIP_MAX_SERIALIZED_SIZE_U64(count) static_cast<uint16_t>(BASE64_ENCODED_LEN(sizeof(uint64_t) * (count)) + 1)
#include <support/Span.h>

namespace chip {

Expand All @@ -55,40 +51,50 @@ class SerializableU64SetBase

/**
* @brief
* Serialize the sparse array into a base 64 encoded string.
* Serialize the sparse array by calling a callback with a ByteSpan to
* serialize. We ensure that this ByteSpan is architecture-agnostic, so
* it can be deserialized anywhere later.
*
* Only the values till mNextAvailable index are encoded.
* The empty indexes between 0, and mNextAvailable, are also
* encoded.
*
* @param[in] buf Buffer where serialized string is written
* @param[in] buflen Length of buf
* @return pointer to buf, or nullptr in case of error
* @param[in] callback the serialization callback to call.
*/
const char * SerializeBase64(char * buf, uint16_t & buflen);
template <typename F>
CHIP_ERROR Serialize(F callback)
{
// Ensure that we are holding little-endian data while the serialization
// callback runs.
SwapByteOrderIfNeeded();

CHIP_ERROR err = callback(ByteSpan(reinterpret_cast<uint8_t *>(mData), SerializedSize()));

SwapByteOrderIfNeeded();
return err;
}

/**
* @brief
* Deserialize a base64 encoded string into the sparse array.
* Deserialize a previously serialized byte buffer into the sparse array.
* The mNextAvailable index is calculated based on how many
* values are in the deserialized array.
*
* @param[in] serialized Serialized buffer
* @param[in] buflen Length of buffer
* @return CHIP_NO_ERROR in case of success, or the error code
*/
CHIP_ERROR DeserializeBase64(const char * serialized, uint16_t buflen);
CHIP_ERROR Deserialize(ByteSpan serialized);

/**
* @brief
* Get the length of string if the array is serialized.
* Get the length of the byte data if the array is serialized.
*/
uint16_t SerializedSize() { return CHIP_MAX_SERIALIZED_SIZE_U64(mNextAvailable); }
size_t SerializedSize() { return sizeof(uint64_t) * mNextAvailable; }

/**
* @brief
* Get the maximum length of string if the array were full and serialized.
* Get the maximum length of the byte data if the array were full and serialized.
*/
uint16_t MaxSerializedSize() { return CHIP_MAX_SERIALIZED_SIZE_U64(mCapacity); }
size_t MaxSerializedSize() { return sizeof(uint64_t) * mCapacity; }

/**
* @brief
Expand Down
41 changes: 21 additions & 20 deletions src/lib/support/tests/TestSerializableIntegerSet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void TestSerializableIntegerSet(nlTestSuite * inSuite, void * inContext)
}

set.Remove(8);
NL_TEST_ASSERT(inSuite, set.SerializedSize() == CHIP_MAX_SERIALIZED_SIZE_U64(0));
NL_TEST_ASSERT(inSuite, set.SerializedSize() == 0);
}

void TestSerializableIntegerSetNonZero(nlTestSuite * inSuite, void * inContext)
Expand Down Expand Up @@ -107,7 +107,7 @@ void TestSerializableIntegerSetNonZero(nlTestSuite * inSuite, void * inContext)
}

set.Remove(7);
NL_TEST_ASSERT(inSuite, set.SerializedSize() == CHIP_MAX_SERIALIZED_SIZE_U64(0));
NL_TEST_ASSERT(inSuite, set.SerializedSize() == 0);
}

void TestSerializableIntegerSetSerialize(nlTestSuite * inSuite, void * inContext)
Expand All @@ -119,29 +119,30 @@ void TestSerializableIntegerSetSerialize(nlTestSuite * inSuite, void * inContext
NL_TEST_ASSERT(inSuite, set.Insert(i) == CHIP_NO_ERROR);
}

char * buf = nullptr;
uint16_t size = 0;

NL_TEST_ASSERT(inSuite, set.SerializeBase64(buf, size) == nullptr);
NL_TEST_ASSERT(inSuite, size != 0);

chip::Platform::ScopedMemoryString buf1("", size);
NL_TEST_ASSERT(inSuite, set.SerializeBase64(buf1.Get(), size) == buf1.Get());
NL_TEST_ASSERT(inSuite, size != 0);

uint16_t size2 = static_cast<uint16_t>(2 * size);
chip::Platform::ScopedMemoryString buf2("", size2);
NL_TEST_ASSERT(inSuite, set.SerializeBase64(buf2.Get(), size2) == buf2.Get());
NL_TEST_ASSERT(inSuite, size2 == size);

chip::SerializableU64Set<8> set2;
NL_TEST_ASSERT(inSuite, set2.DeserializeBase64(buf2.Get(), size2) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite, !set.Contains(0));
for (uint64_t i = 1; i <= 6; i++)
{
NL_TEST_ASSERT(inSuite, set.Contains(i));
}
NL_TEST_ASSERT(inSuite, !set.Contains(7));

NL_TEST_ASSERT(inSuite, set.Serialize([&](chip::ByteSpan serialized) -> CHIP_ERROR {
NL_TEST_ASSERT(inSuite, serialized.size() == 48);
return CHIP_NO_ERROR;
}) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite, set.Serialize([&](chip::ByteSpan serialized) -> CHIP_ERROR {
chip::SerializableU64Set<8> set2;
NL_TEST_ASSERT(inSuite, set2.Deserialize(serialized) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite, !set2.Contains(0));
for (uint64_t i = 1; i <= 6; i++)
{
NL_TEST_ASSERT(inSuite, set2.Contains(i));
}
NL_TEST_ASSERT(inSuite, !set2.Contains(7));
return CHIP_NO_ERROR;
}) == CHIP_NO_ERROR);
}

int Setup(void * inContext)
Expand Down

0 comments on commit f72e0b7

Please sign in to comment.