From 40eca604e7d71c71580aa67c89902f6abab21e1d Mon Sep 17 00:00:00 2001 From: Joonhaeng Heo Date: Mon, 17 Apr 2023 10:47:59 +0900 Subject: [PATCH] Modify using uniqur_ptr --- src/darwin/Framework/CHIP/MTRBaseDevice.mm | 85 +++++++++++++--------- src/lib/support/CHIPMem.h | 15 ++++ 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm index 6f1d8ec301bc75..0e8cc503a5113c 100644 --- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm +++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm @@ -76,12 +76,15 @@ NSString * const MTRArrayValueType = @"Array"; class MTRDataValueDictionaryCallbackBridge; +template class BufferedReadClientCallback; +class MTRDataValueDictionaryDecodableType; @interface MTRReadClientContainer : NSObject @property (nonatomic, readwrite) app::ReadClient * readClientPtr; @property (nonatomic, readwrite) app::AttributePathParams * pathParams; @property (nonatomic, readwrite) app::EventPathParams * eventPathParams; @property (nonatomic, readwrite) uint64_t deviceID; +@property (nonatomic, readwrite) BufferedReadClientCallback * callbackPtr; - (void)onDone; @end @@ -158,6 +161,10 @@ static void PurgeReadClientContainers( Platform::MemoryFree(container.eventPathParams); container.eventPathParams = nullptr; } + if (container.callbackPtr != nullptr) { + Platform::Delete(container.callbackPtr); + container.callbackPtr = nullptr; + } } [listToDelete removeAllObjects]; if (completion) { @@ -240,6 +247,10 @@ - (void)onDone Platform::MemoryFree(_eventPathParams); _eventPathParams = nullptr; } + if (_callbackPtr) { + Platform::Delete(_callbackPtr); + _callbackPtr = nullptr; + } PurgeCompletedReadClientContainers(_deviceID); } @@ -259,6 +270,10 @@ - (void)dealloc Platform::MemoryFree(_eventPathParams); _eventPathParams = nullptr; } + if (_callbackPtr) { + Platform::Delete(_callbackPtr); + _callbackPtr = nullptr; + } } @end @@ -748,7 +763,7 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const static void OnSuccessFn(void * context, id value) { DispatchSuccess(context, value); } }; -template class BufferedReadClientCallback final : public app::ReadClient::Callback { +template class BufferedReadClientCallback : public app::ReadClient::Callback { public: using OnSuccessAttributeCallbackType = std::function; @@ -765,11 +780,7 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const OnErrorCallbackType aOnError, OnDoneCallbackType aOnDone, OnSubscriptionEstablishedCallbackType aOnSubscriptionEstablished = nullptr, OnDeviceResubscriptionScheduledCallbackType aOnDeviceResubscriptionScheduled = nullptr) - : mAttributePathParamsList(aAttributePathParamsList) - , mAttributePathParamsSize(aAttributePathParamsSize) - , mEventPathParamsList(aEventPathParamsList) - , mEventPathParamsSize(aEventPathParamsSize) - , mOnAttributeSuccess(aOnAttributeSuccess) + : mOnAttributeSuccess(aOnAttributeSuccess) , mOnEventSuccess(aOnEventSuccess) , mOnError(aOnError) , mOnDone(aOnDone) @@ -777,6 +788,17 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const , mOnDeviceResubscriptionScheduled(aOnDeviceResubscriptionScheduled) , mBufferedReadAdapter(*this) { + if (aAttributePathParamsList != nullptr) { + mAttributePathParamsList = Platform::MakeMemoryUnique(aAttributePathParamsSize, sizeof(AttributePathParams)); + memcpy(mAttributePathParamsList.get(), aAttributePathParamsList, aAttributePathParamsSize * sizeof(AttributePathParams)); + mAttributePathParamsSize = aAttributePathParamsSize; + } + + if (aEventPathParamsList != nullptr) { + mEventPathParamsList = Platform::MakeMemoryUnique(aEventPathParamsSize, sizeof(EventPathParams)); + memcpy(mEventPathParamsList.get(), aEventPathParamsList, aEventPathParamsSize * sizeof(EventPathParams)); + mEventPathParamsSize = aEventPathParamsSize; + } } ~BufferedReadClientCallback() @@ -784,6 +806,8 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const // Ensure we release the ReadClient before we tear down anything else, // so it can call our OnDeallocatePaths properly. mReadClient = nullptr; + mAttributePathParamsList = nullptr; + mEventPathParamsList = nullptr; } app::BufferedReadCallback & GetBufferedCallback() { return mBufferedReadAdapter; } @@ -809,9 +833,9 @@ void OnAttributeData( VerifyOrExit(aStatus.IsSuccess(), err = aStatus.ToChipError()); VerifyOrExit( - std::find_if(mAttributePathParamsList, mAttributePathParamsList + mAttributePathParamsSize, + std::find_if(mAttributePathParamsList.get(), mAttributePathParamsList.get() + mAttributePathParamsSize, [aPath](app::AttributePathParams & pathParam) -> bool { return pathParam.IsAttributePathSupersetOf(aPath); }) - != mAttributePathParamsList + mAttributePathParamsSize, + != mAttributePathParamsList.get() + mAttributePathParamsSize, err = CHIP_ERROR_SCHEMA_MISMATCH); VerifyOrExit(apData != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); @@ -834,11 +858,11 @@ void OnEventData(const EventHeader & aEventHeader, TLV::TLVReader * apData, cons VerifyOrExit(mEventPathParamsList != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrExit(std::find_if(mEventPathParamsList, mEventPathParamsList + mEventPathParamsSize, + VerifyOrExit(std::find_if(mEventPathParamsList.get(), mEventPathParamsList.get() + mEventPathParamsSize, [aEventHeader](app::EventPathParams & pathParam) -> bool { return pathParam.IsEventPathSupersetOf(aEventHeader.mPath); }) - != mEventPathParamsList + mEventPathParamsSize, + != mEventPathParamsList.get() + mEventPathParamsSize, err = CHIP_ERROR_SCHEMA_MISMATCH); VerifyOrExit(apData != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); @@ -887,8 +911,8 @@ void OnDeallocatePaths(chip::app::ReadPrepareParams && aReadPrepareParams) overr OnDeviceResubscriptionScheduledCallbackType mOnDeviceResubscriptionScheduled; app::BufferedReadCallback mBufferedReadAdapter; Platform::UniquePtr mReadClient; - app::AttributePathParams * mAttributePathParamsList; - app::EventPathParams * mEventPathParamsList; + Platform::MemoryUniquePtr mAttributePathParamsList; + Platform::MemoryUniquePtr mEventPathParamsList; size_t mAttributePathParamsSize; size_t mEventPathParamsSize; }; @@ -979,32 +1003,26 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri } }; - AttributePathParams * attributePathParamsList = nullptr; - EventPathParams * eventPathParamsList = nullptr; + Platform::MemoryUniquePtr attributePathParamsList = nullptr; + Platform::MemoryUniquePtr eventPathParamsList = nullptr; if (attributes != nil) { size_t count = 0; attributePathParamsList - = static_cast(Platform::MemoryCalloc([attributes count], sizeof(AttributePathParams))); + = Platform::MakeMemoryUnique([attributes count], sizeof(AttributePathParams)); VerifyOrReturnError(attributePathParamsList != nullptr, CHIP_ERROR_NO_MEMORY); for (MTRAttributeRequestPath * attribute in attributes) { - [attribute convertToAttributePathParams:attributePathParamsList[count++]]; + [attribute convertToAttributePathParams:attributePathParamsList.get()[count++]]; } } if (events != nil) { size_t count = 0; eventPathParamsList - = static_cast(Platform::MemoryCalloc([events count], sizeof(EventPathParams))); - if (eventPathParamsList == nullptr) { - if (attributePathParamsList != nullptr) { - Platform::MemoryFree(attributePathParamsList); - attributePathParamsList = nullptr; - } - return CHIP_ERROR_NO_MEMORY; - } + = Platform::MakeMemoryUnique([events count], sizeof(EventPathParams)); + VerifyOrReturnError(eventPathParamsList != nullptr, CHIP_ERROR_NO_MEMORY); for (MTREventRequestPath * event in events) { - [event convertToEventPathParams:eventPathParamsList[count++]]; + [event convertToEventPathParams:eventPathParamsList.get()[count++]]; } } @@ -1013,13 +1031,12 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri chip::app::ReadPrepareParams readParams(session); [params toReadPrepareParams:readParams]; - readParams.mpAttributePathParamsList = attributePathParamsList; + readParams.mpAttributePathParamsList = attributePathParamsList.get(); readParams.mAttributePathParamsListSize = [attributePaths count]; - readParams.mpEventPathParamsList = eventPathParamsList; + readParams.mpEventPathParamsList = eventPathParamsList.get(); readParams.mEventPathParamsListSize = [eventPaths count]; - auto onDone = [resultArray, interactionStatus, bridge, successCb, failureCb, attributePathParamsList, - eventPathParamsList](BufferedReadClientCallback * callback) { + auto onDone = [resultArray, interactionStatus, bridge, successCb, failureCb](BufferedReadClientCallback * callback) { if (*interactionStatus != CHIP_NO_ERROR) { // Failure failureCb(bridge, *interactionStatus); @@ -1027,17 +1044,11 @@ - (void)readAttributePaths:(NSArray * _Nullable)attri // Success successCb(bridge, resultArray); } - if (attributePathParamsList != nullptr) { - Platform::MemoryFree(attributePathParamsList); - } - if (eventPathParamsList != nullptr) { - Platform::MemoryFree(eventPathParamsList); - } chip::Platform::Delete(callback); }; auto callback = chip::Platform::MakeUnique>( - attributePathParamsList, readParams.mAttributePathParamsListSize, eventPathParamsList, + attributePathParamsList.get(), readParams.mAttributePathParamsListSize, eventPathParamsList.get(), readParams.mEventPathParamsListSize, onAttributeSuccessCb, onEventSuccessCb, onFailureCb, onDone, nullptr); VerifyOrReturnError(callback != nullptr, CHIP_ERROR_NO_MEMORY); @@ -1437,6 +1448,7 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl readParams.mEventPathParamsListSize = eventPathSize; auto onDone = [container](BufferedReadClientCallback * callback) { + container.callbackPtr = nullptr; [container onDone]; // Make sure we delete callback last, because doing that actually destroys our // lambda, so we can't access captured values after that. @@ -1477,6 +1489,7 @@ - (void)subscribeToAttributePaths:(NSArray * _Nullabl // Read clients will be purged when deregistered. container.readClientPtr = readClient; + container.callbackPtr = callback.get(); AddReadClientContainer(container.deviceID, container); callback.release(); }]; diff --git a/src/lib/support/CHIPMem.h b/src/lib/support/CHIPMem.h index a1e27c20b73ce3..463366de3c92c3 100644 --- a/src/lib/support/CHIPMem.h +++ b/src/lib/support/CHIPMem.h @@ -175,15 +175,30 @@ struct Deleter void operator()(T * p) { Delete(p); } }; +template +struct MemoryFreer +{ + void operator()(T * p) { MemoryFree(p); } +}; + template using UniquePtr = std::unique_ptr>; +template +using MemoryUniquePtr = std::unique_ptr>; + template inline UniquePtr MakeUnique(Args &&... args) { return UniquePtr(New(std::forward(args)...)); } +template +inline MemoryUniquePtr MakeMemoryUnique(size_t num, size_t size) +{ + return MemoryUniquePtr(static_cast(MemoryCalloc(num, size))); +} + template using SharedPtr = std::shared_ptr;