Skip to content

Commit

Permalink
Group Data Provider: Code review corrections.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rcasallas-silabs
rcasallas-silabs committed Oct 12, 2021
1 parent 696d409 commit 105a080
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 27 deletions.
46 changes: 33 additions & 13 deletions src/credentials/examples/GroupDataProviderExample.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,24 @@ class StaticGroupsProvider : public GroupDataProvider
struct EndpointEntry : public GroupMapping
{
bool in_use = false;
void Clear()
{
endpoint = 0;
group = 0;
in_use = false;
}
};

struct KeysEntry : public KeySet
{
bool in_use = false;
uint16_t key_set_index;
void Clear()
{
memset(epoch_keys, 0x00, sizeof(epoch_keys));
key_set_index = 0;
num_keys_used = 0;
in_use = false;
}
};

struct Fabric
Expand All @@ -63,8 +75,14 @@ class StaticGroupsProvider : public GroupDataProvider
in_use = false;
fabric_index = 0;
states_count = 0;
memset(endpoints, 0x00, sizeof(endpoints));
memset(keys, 0x00, sizeof(keys));
for (size_t i = 0; i < kEndpointEntriesMax; i++)
{
endpoints[i].Clear();
}
for (size_t i = 0; i < kKeyEntriesMax; i++)
{
keys[i].Clear();
}
}
};

Expand Down Expand Up @@ -324,7 +342,7 @@ class StaticGroupsProvider : public GroupDataProvider
VerifyOrReturnError(mInitialized, false);

Fabric * fabric = GetExistingFabric(fabric_index);
VerifyOrReturnError(fabric, false);
VerifyOrReturnError(nullptr != fabric, false);

for (uint16_t i = 0; fabric && i < kEndpointEntriesMax; ++i)
{
Expand All @@ -344,7 +362,7 @@ class StaticGroupsProvider : public GroupDataProvider
(void) name; // Unused!

Fabric * fabric = GetExistingFabricOrAllocateNew(fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_NO_MEMORY);
VerifyOrReturnError(nullptr != fabric, CHIP_ERROR_NO_MEMORY);

// Search for existing mapping
for (uint16_t i = 0; i < kEndpointEntriesMax; ++i)
Expand Down Expand Up @@ -376,7 +394,7 @@ class StaticGroupsProvider : public GroupDataProvider
VerifyOrReturnError(mInitialized, CHIP_ERROR_INTERNAL);

Fabric * fabric = GetExistingFabric(fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_INVALID_FABRIC_ID);
VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_FABRIC_ID);

// Search for existing mapping
for (uint16_t i = 0; fabric && i < kEndpointEntriesMax; ++i)
Expand All @@ -398,7 +416,7 @@ class StaticGroupsProvider : public GroupDataProvider
VerifyOrReturnError(mInitialized, CHIP_ERROR_INTERNAL);

Fabric * fabric = GetExistingFabric(fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_INVALID_FABRIC_ID);
VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_FABRIC_ID);

// Remove all mappings from fabric
for (uint16_t i = 0; fabric && i < kEndpointEntriesMax; ++i)
Expand Down Expand Up @@ -426,7 +444,7 @@ class StaticGroupsProvider : public GroupDataProvider
VerifyOrReturnError(static_cast<size_t>(state_index) <= mGroupStatesCount, CHIP_ERROR_INVALID_ARGUMENT);

Fabric * fabric = GetExistingFabricOrAllocateNew(state.fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_NO_MEMORY);
VerifyOrReturnError(nullptr != fabric, CHIP_ERROR_NO_MEMORY);

StateEntry & entry = mGroupStates[state_index];
bool appending = static_cast<size_t>(state_index) == mGroupStatesCount;
Expand Down Expand Up @@ -538,7 +556,7 @@ class StaticGroupsProvider : public GroupDataProvider
Fabric * fabric = GetExistingFabricOrAllocateNew(fabric_index);
KeysEntry * entry = nullptr;

VerifyOrReturnError(fabric, CHIP_ERROR_INVALID_FABRIC_ID);
VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_FABRIC_ID);

// Search for existing, or unused entry
for (uint16_t i = 0; fabric && i < kKeyEntriesMax; ++i)
Expand Down Expand Up @@ -574,7 +592,7 @@ class StaticGroupsProvider : public GroupDataProvider
{
VerifyOrReturnError(mInitialized, CHIP_ERROR_INTERNAL);
Fabric * fabric = GetExistingFabric(fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_INVALID_FABRIC_ID);
VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_FABRIC_ID);

// Search for existing keys
for (uint16_t i = 0; fabric && i < kKeyEntriesMax; ++i)
Expand All @@ -596,7 +614,7 @@ class StaticGroupsProvider : public GroupDataProvider
{
VerifyOrReturnError(mInitialized, CHIP_ERROR_INTERNAL);
Fabric * fabric = GetExistingFabric(fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_INVALID_FABRIC_ID);
VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_FABRIC_ID);

// Search for existing keys
for (uint16_t i = 0; fabric && i < kKeyEntriesMax; ++i)
Expand All @@ -623,7 +641,7 @@ class StaticGroupsProvider : public GroupDataProvider
CHIP_ERROR RemoveFabric(chip::FabricIndex fabric_index) override
{
Fabric * fabric = GetExistingFabric(fabric_index);
VerifyOrReturnError(fabric, CHIP_ERROR_INVALID_FABRIC_ID);
VerifyOrReturnError(fabric != nullptr, CHIP_ERROR_INVALID_FABRIC_ID);
// Remove group states
for (size_t i = 0; i < kMaxNumGroupStates; ++i)
{
Expand All @@ -632,7 +650,9 @@ class StaticGroupsProvider : public GroupDataProvider
RemoveGroupState(i);
}
}
// Release fabric entry
// Release other resources associated with the fabric entry, such as mapped
// endpoints (i.e. through the Groups cluster) and group key sets, both of which
// live in the `Fabric` data structure of this implementation.
fabric->Clear();
return CHIP_NO_ERROR;
}
Expand Down
37 changes: 23 additions & 14 deletions src/credentials/tests/TestGroupDataProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,13 @@ void TestKeys(nlTestSuite * apSuite, void * apContext)
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys0b.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys0b.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys0b.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite, 0 == memcmp(keys0a.epoch_keys, keys0b.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

err = groups->GetKeySet(kFabricIndex, 1, keys1b);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys1a.policy == keys1b.policy);
NL_TEST_ASSERT(apSuite, keys1a.num_keys_used == keys1b.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys1a.epoch_keys, keys1b.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));
NL_TEST_ASSERT(apSuite, 0 == memcmp(keys1a.epoch_keys, keys1b.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));

err = groups->GetKeySet(kFabricIndex, 3, keys3);
NL_TEST_ASSERT(apSuite, CHIP_ERROR_KEY_NOT_FOUND == err);
Expand Down Expand Up @@ -579,7 +579,7 @@ void TestKeysIterator(nlTestSuite * apSuite, void * apContext)
groups->Finish();
}

void TestFabrics(nlTestSuite * apSuite, void * apContext)
void TestPerFabricData(nlTestSuite * apSuite, void * apContext)
{
GroupDataProvider * groups = GetGroupDataProvider();
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == groups->Init());
Expand Down Expand Up @@ -714,37 +714,43 @@ void TestFabrics(nlTestSuite * apSuite, void * apContext)
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

err = groups->GetKeySet(kFabricIndex2, 303, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys1a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys1a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys1a.epoch_keys, keys_out.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys1a.epoch_keys, keys_out.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));

err = groups->GetKeySet(kFabricIndex2, 505, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

err = groups->GetKeySet(kFabricIndex1, 202, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

err = groups->GetKeySet(kFabricIndex1, 404, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys1a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys1a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys1a.epoch_keys, keys_out.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys1a.epoch_keys, keys_out.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));

err = groups->GetKeySet(kFabricIndex1, 606, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

// Remove Fabric
err = groups->RemoveFabric(kFabricIndex1);
Expand All @@ -770,7 +776,7 @@ void TestFabrics(nlTestSuite * apSuite, void * apContext)
exists = groups->GroupMappingExists(kFabricIndex2, group1c);
NL_TEST_ASSERT(apSuite, exists);

// States
// States: Removing the fabric shift the remaining groups states to a lower index

err = groups->GetGroupState(0, state0b);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
Expand All @@ -793,19 +799,22 @@ void TestFabrics(nlTestSuite * apSuite, void * apContext)
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

err = groups->GetKeySet(kFabricIndex2, 303, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys1a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys1a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys1a.epoch_keys, keys_out.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys1a.epoch_keys, keys_out.epoch_keys, sizeof(keys1a.epoch_keys[0]) * keys1a.num_keys_used));

err = groups->GetKeySet(kFabricIndex2, 505, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_NO_ERROR == err);
NL_TEST_ASSERT(apSuite, keys0a.policy == keys_out.policy);
NL_TEST_ASSERT(apSuite, keys0a.num_keys_used == keys_out.num_keys_used);
NL_TEST_ASSERT(apSuite, !memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));
NL_TEST_ASSERT(apSuite,
0 == memcmp(keys0a.epoch_keys, keys_out.epoch_keys, sizeof(keys0a.epoch_keys[0]) * keys0a.num_keys_used));

err = groups->GetKeySet(kFabricIndex1, 202, keys_out);
NL_TEST_ASSERT(apSuite, CHIP_ERROR_INVALID_FABRIC_ID == err);
Expand Down Expand Up @@ -855,7 +864,7 @@ const nlTest sTests[] = { NL_TEST_DEF("TestEndpoints", chip::app::TestGroups::Te
NL_TEST_DEF("TestStateIterator", chip::app::TestGroups::TestStateIterator),
NL_TEST_DEF("TestKeys", chip::app::TestGroups::TestKeys),
NL_TEST_DEF("TestKeysIterator", chip::app::TestGroups::TestKeysIterator),
NL_TEST_DEF("TestFabrics", chip::app::TestGroups::TestFabrics),
NL_TEST_DEF("TestPerFabricData", chip::app::TestGroups::TestPerFabricData),
NL_TEST_SENTINEL() };
} // namespace

Expand Down

0 comments on commit 105a080

Please sign in to comment.