From ebff9bb6e711cccb05bd564411f9bdfa34472b73 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 17 Oct 2024 12:29:41 -0400 Subject: [PATCH 1/5] Fix up group iterator releasing --- src/app/WriteHandler.cpp | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index d2fde339304f8f..4e57df7f0a6a2a 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -42,6 +42,29 @@ namespace chip { namespace app { +namespace { + +class AutoReleaseIterator +{ +public: + AutoReleaseIterator(Credentials::GroupDataProvider::EndpointIterator * iterator) : mIterator(iterator) {} + ~AutoReleaseIterator() + { + if (mIterator != nullptr) + { + mIterator->Release(); + } + } + + bool IsNull() const { return mIterator == nullptr; } + bool Next(Credentials::GroupDataProvider::GroupEndpoint & item) { return mIterator->Next(item); } + +private: + Credentials::GroupDataProvider::EndpointIterator * mIterator; +}; + +} // namespace + using namespace Protocols::InteractionModel; using Status = Protocols::InteractionModel::Status; @@ -436,10 +459,6 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut ConcreteDataAttributePath dataAttributePath; TLV::TLVReader reader = aAttributeDataIBsReader; - Credentials::GroupDataProvider::GroupEndpoint mapping; - Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider(); - Credentials::GroupDataProvider::EndpointIterator * iterator; - err = element.Init(reader); SuccessOrExit(err); @@ -461,8 +480,8 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut "Received group attribute write for Group=%u Cluster=" ChipLogFormatMEI " attribute=" ChipLogFormatMEI, groupId, ChipLogValueMEI(dataAttributePath.mClusterId), ChipLogValueMEI(dataAttributePath.mAttributeId)); - iterator = groupDataProvider->IterateEndpoints(fabric); - VerifyOrExit(iterator != nullptr, err = CHIP_ERROR_NO_MEMORY); + AutoReleaseIterator iterator(Credentials::GetGroupDataProvider()->IterateEndpoints(fabric); + VerifyOrExit(!iterator.IsNull(), err = CHIP_ERROR_NO_MEMORY); bool shouldReportListWriteEnd = ShouldReportListWriteEnd( mProcessingAttributePath, mStateFlags.Has(StateBits::kProcessingAttributeIsList), dataAttributePath); @@ -470,7 +489,8 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut std::optional isListAttribute = std::nullopt; - while (iterator->Next(mapping)) + Credentials::GroupDataProvider::GroupEndpoint mapping; + while (iterator.Next(mapping)) { if (groupId != mapping.group_id) { @@ -552,7 +572,6 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut dataAttributePath.mEndpointId = kInvalidEndpointId; mStateFlags.Set(StateBits::kProcessingAttributeIsList, dataAttributePath.IsListOperation()); mProcessingAttributePath.SetValue(dataAttributePath); - iterator->Release(); } if (CHIP_END_OF_TLV == err) From 56855bd02a7875a113015943fda2e796c687352b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 17 Oct 2024 12:33:02 -0400 Subject: [PATCH 2/5] Add missing bracket --- src/app/WriteHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 4e57df7f0a6a2a..244fe5eda3c918 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -480,7 +480,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut "Received group attribute write for Group=%u Cluster=" ChipLogFormatMEI " attribute=" ChipLogFormatMEI, groupId, ChipLogValueMEI(dataAttributePath.mClusterId), ChipLogValueMEI(dataAttributePath.mAttributeId)); - AutoReleaseIterator iterator(Credentials::GetGroupDataProvider()->IterateEndpoints(fabric); + AutoReleaseIterator iterator(Credentials::GetGroupDataProvider()->IterateEndpoints(fabric)); VerifyOrExit(!iterator.IsNull(), err = CHIP_ERROR_NO_MEMORY); bool shouldReportListWriteEnd = ShouldReportListWriteEnd( From 4c7fd3b752acc1634323f4926b66457270d7d774 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 17 Oct 2024 12:34:40 -0400 Subject: [PATCH 3/5] Renames and comments --- src/app/WriteHandler.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 244fe5eda3c918..e4bebe227729e1 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -44,11 +44,13 @@ namespace app { namespace { -class AutoReleaseIterator +/// Wraps a EndpointIterator and ensures that `::Release()` is called +/// for the iterator (assuming it is non-null) +class AutoReleaseGroupEndpointIterator { public: - AutoReleaseIterator(Credentials::GroupDataProvider::EndpointIterator * iterator) : mIterator(iterator) {} - ~AutoReleaseIterator() + AutoReleaseGroupEndpointIterator(Credentials::GroupDataProvider::EndpointIterator * iterator) : mIterator(iterator) {} + ~AutoReleaseGroupEndpointIterator() { if (mIterator != nullptr) { @@ -58,7 +60,6 @@ class AutoReleaseIterator bool IsNull() const { return mIterator == nullptr; } bool Next(Credentials::GroupDataProvider::GroupEndpoint & item) { return mIterator->Next(item); } - private: Credentials::GroupDataProvider::EndpointIterator * mIterator; }; @@ -480,7 +481,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut "Received group attribute write for Group=%u Cluster=" ChipLogFormatMEI " attribute=" ChipLogFormatMEI, groupId, ChipLogValueMEI(dataAttributePath.mClusterId), ChipLogValueMEI(dataAttributePath.mAttributeId)); - AutoReleaseIterator iterator(Credentials::GetGroupDataProvider()->IterateEndpoints(fabric)); + AutoReleaseGroupEndpointIterator iterator(Credentials::GetGroupDataProvider()->IterateEndpoints(fabric)); VerifyOrExit(!iterator.IsNull(), err = CHIP_ERROR_NO_MEMORY); bool shouldReportListWriteEnd = ShouldReportListWriteEnd( From 9f6ed73cf5483b69250fbc31e3297fec3d4b6fb1 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 17 Oct 2024 16:35:18 +0000 Subject: [PATCH 4/5] Restyled by clang-format --- src/app/WriteHandler.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index e4bebe227729e1..5b7536ad6df933 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -60,6 +60,7 @@ class AutoReleaseGroupEndpointIterator bool IsNull() const { return mIterator == nullptr; } bool Next(Credentials::GroupDataProvider::GroupEndpoint & item) { return mIterator->Next(item); } + private: Credentials::GroupDataProvider::EndpointIterator * mIterator; }; From dac20cc9000d6b29c91d2288617774e4df605162 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 17 Oct 2024 14:19:50 -0400 Subject: [PATCH 5/5] Update src/app/WriteHandler.cpp Co-authored-by: Arkadiusz Bokowy --- src/app/WriteHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 5b7536ad6df933..011fd2e46b48ab 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -49,7 +49,7 @@ namespace { class AutoReleaseGroupEndpointIterator { public: - AutoReleaseGroupEndpointIterator(Credentials::GroupDataProvider::EndpointIterator * iterator) : mIterator(iterator) {} + explicit AutoReleaseGroupEndpointIterator(Credentials::GroupDataProvider::EndpointIterator * iterator) : mIterator(iterator) {} ~AutoReleaseGroupEndpointIterator() { if (mIterator != nullptr)