From fe8e6d8eba2dc599e837c1580ee9b885a9cb9526 Mon Sep 17 00:00:00 2001 From: jepenven-silabs Date: Mon, 7 Mar 2022 15:21:08 -0500 Subject: [PATCH] Apply PR comments --- src/app/server/Server.cpp | 4 +- src/app/server/Server.h | 16 ++++--- .../CHIPDeviceControllerFactory.cpp | 2 +- src/controller/CHIPDeviceControllerFactory.h | 16 ++++--- src/credentials/FabricTable.cpp | 11 ++--- src/credentials/FabricTable.h | 42 +++++-------------- 6 files changed, 37 insertions(+), 54 deletions(-) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 15798fd83ef57f..1dfb12e6d77a30 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -184,9 +184,9 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint err = mSessions.Init(&DeviceLayer::SystemLayer(), &mTransports, &mMessageCounterManager, &mDeviceStorage, &GetFabricTable()); SuccessOrExit(err); - err = mFabricListener.Init(&mSessions); + err = mFabricDelegate.Init(&mSessions); SuccessOrExit(err); - mFabrics.SetListener(&mFabricListener); + mFabrics.AddFabricDelegate(&mFabricDelegate); err = mExchangeMgr.Init(&mSessions); SuccessOrExit(err); diff --git a/src/app/server/Server.h b/src/app/server/Server.h index 29330cd50e71fd..bde206375d033f 100644 --- a/src/app/server/Server.h +++ b/src/app/server/Server.h @@ -182,10 +182,10 @@ class Server ServerTransportMgr * mTransports; }; - class FabricTableListener final : public FabricTable::FabricListener + class ServerFabricDelegate final : public FabricTableDelegate { public: - FabricTableListener() {} + ServerFabricDelegate() {} CHIP_ERROR Init(SessionManager * sessionManager) { @@ -195,18 +195,22 @@ class Server return CHIP_NO_ERROR; }; - void OnFabricRemoved(FabricIndex fabric_index) override + void OnFabricDeletedFromStorage(CompressedFabricId compressedId, FabricIndex fabricIndex) override { + (void) compressedId; if (mSessionManager != nullptr) { - mSessionManager->SyncRemovalFabricIndex(fabric_index); + mSessionManager->SyncRemovalFabricIndex(fabricIndex); } Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider(); if (groupDataProvider != nullptr) { - groupDataProvider->RemoveFabric(fabric_index); + groupDataProvider->RemoveFabric(fabricIndex); } }; + void OnFabricRetrievedFromStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; } + + void OnFabricPersistedToStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; } private: SessionManager * mSessionManager = nullptr; @@ -240,7 +244,7 @@ class Server Credentials::GroupDataProviderImpl mGroupsProvider; app::DefaultAttributePersistenceProvider mAttributePersister; GroupDataProviderListener mListener; - FabricTableListener mFabricListener; + ServerFabricDelegate mFabricDelegate; Access::AccessControl mAccessControl; diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp index f54b90426c3943..20d8665863363e 100644 --- a/src/controller/CHIPDeviceControllerFactory.cpp +++ b/src/controller/CHIPDeviceControllerFactory.cpp @@ -146,7 +146,7 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params) stateParams.messageCounterManager = chip::Platform::New(); ReturnErrorOnFailure( - stateParams.fabricTable->Init(mFabricStorage, chip::Platform::New(stateParams.sessionMgr))); + stateParams.fabricTable->Init(mFabricStorage, chip::Platform::New(stateParams.sessionMgr))); ReturnErrorOnFailure(stateParams.sessionMgr->Init(stateParams.systemLayer, stateParams.transportMgr, stateParams.messageCounterManager, params.fabricIndependentStorage, diff --git a/src/controller/CHIPDeviceControllerFactory.h b/src/controller/CHIPDeviceControllerFactory.h index cac3798d9aadce..4fcbb9df86c6f4 100644 --- a/src/controller/CHIPDeviceControllerFactory.h +++ b/src/controller/CHIPDeviceControllerFactory.h @@ -139,11 +139,11 @@ class DeviceControllerFactory // void ReleaseSystemState() { mSystemState->Release(); } - class FabricTableListener final : public FabricTable::FabricListener + class ControllerFabricDelegate final : public FabricTableDelegate { public: - FabricTableListener() {} - FabricTableListener(SessionManager * sessionManager) : mSessionManager(sessionManager) {} + ControllerFabricDelegate() {} + ControllerFabricDelegate(SessionManager * sessionManager) : FabricTableDelegate(true), mSessionManager(sessionManager) {} CHIP_ERROR Init(SessionManager * sessionManager) { @@ -153,19 +153,23 @@ class DeviceControllerFactory return CHIP_NO_ERROR; }; - void OnFabricRemoved(FabricIndex fabric_index) override + void OnFabricDeletedFromStorage(CompressedFabricId compressedId, FabricIndex fabricIndex) override { if (mSessionManager != nullptr) { - mSessionManager->SyncRemovalFabricIndex(fabric_index); + mSessionManager->SyncRemovalFabricIndex(fabricIndex); } Credentials::GroupDataProvider * groupDataProvider = Credentials::GetGroupDataProvider(); if (groupDataProvider != nullptr) { - groupDataProvider->RemoveFabric(fabric_index); + groupDataProvider->RemoveFabric(fabricIndex); } }; + void OnFabricRetrievedFromStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; } + + void OnFabricPersistedToStorage(FabricInfo * fabricInfo) override { (void) fabricInfo; } + private: SessionManager * mSessionManager = nullptr; }; diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp index f62e759fd7d8b2..5c953bcd0787e5 100644 --- a/src/credentials/FabricTable.cpp +++ b/src/credentials/FabricTable.cpp @@ -629,10 +629,6 @@ CHIP_ERROR FabricTable::Delete(FabricIndex index) mFabricCount--; } ChipLogProgress(Discovery, "Fabric (%d) deleted. Calling OnFabricDeletedFromStorage", index); - if (mFabricListener != nullptr) - { - mFabricListener->OnFabricRemoved(index); - } FabricTableDelegate * delegate = mDelegate; while (delegate) @@ -653,13 +649,12 @@ void FabricTable::DeleteAllFabrics() } } -CHIP_ERROR FabricTable::Init(FabricStorage * storage, FabricListener * fabricListener) +CHIP_ERROR FabricTable::Init(FabricStorage * storage, FabricTableDelegate * fabricTableDelegate) { VerifyOrReturnError(storage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - if (fabricListener != nullptr) + if (fabricTableDelegate != nullptr) { - mFabricListener = std::move(fabricListener); - mlistenerOwneship = true; + ReturnErrorOnFailure(AddFabricDelegate(fabricTableDelegate)); } mStorage = storage; diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h index cdcc65a6e4f215..c375aa863c37cc 100644 --- a/src/credentials/FabricTable.h +++ b/src/credentials/FabricTable.h @@ -329,6 +329,7 @@ class DLL_EXPORT FabricTableDelegate friend class FabricTable; public: + FabricTableDelegate(bool selfOwned = false) : mSelfOwned(selfOwned) {} virtual ~FabricTableDelegate() {} /** * Gets called when a fabric is deleted from KVS store. @@ -347,6 +348,7 @@ class DLL_EXPORT FabricTableDelegate private: FabricTableDelegate * mNext = nullptr; + bool mSelfOwned = false; }; /** @@ -422,9 +424,15 @@ class DLL_EXPORT FabricTable FabricTable() { Reset(); } ~FabricTable() { - if (mlistenerOwneship) + FabricTableDelegate * delegate = mDelegate; + while (delegate) { - chip::Platform::MemoryFree(mFabricListener); + FabricTableDelegate * temp = delegate->mNext; + if (delegate->mSelfOwned) + { + chip::Platform::Delete(delegate); + } + delegate = temp; } } CHIP_ERROR Store(FabricIndex index); @@ -456,29 +464,7 @@ class DLL_EXPORT FabricTable void Reset(); - /** - * Interface to listen for changes in FabricRemoval. - */ - class FabricListener - { - public: - virtual ~FabricListener() = default; - /** - * Callback invoked when an existing fabric is removed. - * - * @param[in] fabric_index Fabric Index of the removed fabric. - */ - virtual void OnFabricRemoved(FabricIndex fabric_index) = 0; - }; - - /** - * @brief Initialize the fabric table - * @arg FabricStorage * Pointer to the FabricStorage delegate for persistence - * @arg FabricListener * Transfert ownership of the listener pointer to the fabric table - * To prevent transfert of ownership, use SetListener method. - * @return CHIP_ERROR error code - */ - CHIP_ERROR Init(FabricStorage * storage, FabricListener * fabricListener); + CHIP_ERROR Init(FabricStorage * storage, FabricTableDelegate * fabricTableDelegate); CHIP_ERROR AddFabricDelegate(FabricTableDelegate * delegate); uint8_t FabricCount() const { return mFabricCount; } @@ -488,20 +474,14 @@ class DLL_EXPORT FabricTable ConstFabricIterator begin() const { return cbegin(); } ConstFabricIterator end() const { return cend(); } - void SetListener(FabricListener * fabricListener) { mFabricListener = fabricListener; } - private: FabricInfo mStates[CHIP_CONFIG_MAX_FABRICS]; FabricStorage * mStorage = nullptr; FabricTableDelegate * mDelegate = nullptr; - FabricListener * mFabricListener = nullptr; - FabricIndex mNextAvailableFabricIndex = kMinValidFabricIndex; uint8_t mFabricCount = 0; - - bool mlistenerOwneship = false; }; } // namespace chip