From 4aa5307bf466fdfed85e217afacf2b0c46440df9 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 10 Mar 2022 12:27:40 -0500 Subject: [PATCH 1/6] Allow runtime init of some `Server` members (2/2) - This PR is on the path towards having Server::Server no longer statically initialize its members with storage, and instead relying on Server::Init(). This will simplify organization of unit tests and also the convergence of Controller/Server storage to address issue #16028 Issue #16028 This PR separates init of AccessControl so that the delegate need not be passed to the Constructor, which allows simpler splitting of storage init and decoupling from Server::Server() static inializer list. This is a structural, non-functional change, touching only AccessControl. Testing - Ran cert tests, still pass - Ran unit tests, still pass --- src/access/AccessControl.cpp | 25 +++++-- src/access/AccessControl.h | 67 +++++++++++++++---- .../examples/ExampleAccessControlDelegate.cpp | 4 +- .../examples/ExampleAccessControlDelegate.h | 4 +- .../PermissiveAccessControlDelegate.cpp | 4 +- .../PermissiveAccessControlDelegate.h | 2 +- src/access/tests/TestAccessControl.cpp | 4 +- src/app/server/Server.cpp | 10 ++- src/app/tests/AppTestContext.cpp | 5 +- 9 files changed, 95 insertions(+), 30 deletions(-) diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index b193b0fb0716be..befd33990bf28e 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -165,33 +165,48 @@ AccessControl::Delegate AccessControl::mDefaultDelegate; CHIP_ERROR AccessControl::Init() { + VerifyOrReturnError(!mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + ChipLogProgress(DataManagement, "AccessControl: initializing"); - return mDelegate.Init(); + CHIP_ERROR retval = mDelegate->Init(); + + if (retval == CHIP_NO_ERROR) + { + mIsInitialized = true; + } + return retval; } CHIP_ERROR AccessControl::Finish() { + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); ChipLogProgress(DataManagement, "AccessControl: finishing"); - return mDelegate.Finish(); + CHIP_ERROR retval = mDelegate->Finish(); + + mIsInitialized = false; + return retval; } CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, const RequestPath & requestPath, Privilege requestPrivilege) { + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + #if CHIP_PROGRESS_LOGGING { - char buf[6 * kCharsPerCatForLogging]; + constexpr size_t kMaxCatsToLog = 6; + char catLogBuf[kMaxCatsToLog * kCharsPerCatForLogging]; ChipLogProgress(DataManagement, "AccessControl: checking f=%u a=%c s=0x" ChipLogFormatX64 " t=%s c=" ChipLogFormatMEI " e=%" PRIu16 " p=%c", subjectDescriptor.fabricIndex, GetAuthModeStringForLogging(subjectDescriptor.authMode), ChipLogValueX64(subjectDescriptor.subject), - GetCatStringForLogging(buf, sizeof(buf), subjectDescriptor.cats), ChipLogValueMEI(requestPath.cluster), + GetCatStringForLogging(catLogBuf, sizeof(catLogBuf), subjectDescriptor.cats), ChipLogValueMEI(requestPath.cluster), requestPath.endpoint, GetPrivilegeStringForLogging(requestPrivilege)); } #endif // TODO(#13867): this will go away - if (mDelegate.TemporaryCheckOverride()) + if (mDelegate->TemporaryCheckOverride()) { ChipLogProgress(DataManagement, "AccessControl: temporary check override (this will go away)"); return CHIP_NO_ERROR; diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h index d6f7254fa52d0b..8d8cf2889de578 100644 --- a/src/access/AccessControl.h +++ b/src/access/AccessControl.h @@ -23,6 +23,7 @@ #include "SubjectDescriptor.h" #include +#include "lib/support/CodeUtils.h" namespace chip { namespace Access { @@ -353,12 +354,35 @@ class AccessControl AccessControl() = default; - AccessControl(Delegate & delegate) : mDelegate(delegate) {} - AccessControl(const AccessControl &) = delete; AccessControl & operator=(const AccessControl &) = delete; - ~AccessControl() { mDelegate.Release(); } + ~AccessControl() { mDelegate->Release(); } + + /** + * @brief Set the delegate that will handle access control entries. Must be called before Init(). + * + * @param delegate - The delegate to use + * @return CHIP_NO_ERROR on success, CHIP_ERROR_INCORRECT_STATE if set after Init(), and + * CHIP_ERROR_INVALID_ARGUMENT if delegate is null. + */ + CHIP_ERROR SetDelegate(Delegate * delegate) + { + if (mIsInitialized) + { + return CHIP_ERROR_INCORRECT_STATE; + } + else if (delegate == nullptr) + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + + // De-initialize the default-set delegate. + mDelegate->Release(); + + mDelegate = delegate; + return CHIP_NO_ERROR; + } /** * Initialize the access control module. Must be called before first use. @@ -373,10 +397,18 @@ class AccessControl CHIP_ERROR Finish(); // Capabilities - CHIP_ERROR GetMaxEntryCount(size_t & value) const { return mDelegate.GetMaxEntryCount(value); } + CHIP_ERROR GetMaxEntryCount(size_t & value) const + { + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->GetMaxEntryCount(value); + } // Actualities - CHIP_ERROR GetEntryCount(size_t & value) const { return mDelegate.GetEntryCount(value); } + CHIP_ERROR GetEntryCount(size_t & value) const + { + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->GetEntryCount(value); + } /** * Prepares an entry. @@ -385,7 +417,11 @@ class AccessControl * * @param [in] entry Entry to prepare. */ - CHIP_ERROR PrepareEntry(Entry & entry) { return mDelegate.PrepareEntry(entry); } + CHIP_ERROR PrepareEntry(Entry & entry) + { + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->PrepareEntry(entry); + } /** * Creates an entry in the access control list. @@ -397,7 +433,8 @@ class AccessControl CHIP_ERROR CreateEntry(size_t * index, const Entry & entry, FabricIndex * fabricIndex = nullptr) { ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT); - return mDelegate.CreateEntry(index, entry, fabricIndex); + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->CreateEntry(index, entry, fabricIndex); } /** @@ -409,7 +446,8 @@ class AccessControl */ CHIP_ERROR ReadEntry(size_t index, Entry & entry, const FabricIndex * fabricIndex = nullptr) const { - return mDelegate.ReadEntry(index, entry, fabricIndex); + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->ReadEntry(index, entry, fabricIndex); } /** @@ -422,7 +460,8 @@ class AccessControl CHIP_ERROR UpdateEntry(size_t index, const Entry & entry, const FabricIndex * fabricIndex = nullptr) { ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT); - return mDelegate.UpdateEntry(index, entry, fabricIndex); + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->UpdateEntry(index, entry, fabricIndex); } /** @@ -433,7 +472,8 @@ class AccessControl */ CHIP_ERROR DeleteEntry(size_t index, const FabricIndex * fabricIndex = nullptr) { - return mDelegate.DeleteEntry(index, fabricIndex); + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->DeleteEntry(index, fabricIndex); } /** @@ -444,7 +484,8 @@ class AccessControl */ CHIP_ERROR Entries(EntryIterator & iterator, const FabricIndex * fabricIndex = nullptr) const { - return mDelegate.Entries(iterator, fabricIndex); + VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + return mDelegate->Entries(iterator, fabricIndex); } /** @@ -461,7 +502,9 @@ class AccessControl bool IsValid(const Entry & entry); static Delegate mDefaultDelegate; - Delegate & mDelegate = mDefaultDelegate; + Delegate * mDelegate = &mDefaultDelegate; + + bool mIsInitialized = false; }; /** diff --git a/src/access/examples/ExampleAccessControlDelegate.cpp b/src/access/examples/ExampleAccessControlDelegate.cpp index 53a83edbea48ad..95c659a493c155 100644 --- a/src/access/examples/ExampleAccessControlDelegate.cpp +++ b/src/access/examples/ExampleAccessControlDelegate.cpp @@ -1310,11 +1310,11 @@ namespace chip { namespace Access { namespace Examples { -AccessControl::Delegate & GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate) +AccessControl::Delegate * GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate) { static AccessControlDelegate accessControlDelegate; accessControlDelegate.SetStorageDelegate(storageDelegate); - return accessControlDelegate; + return &accessControlDelegate; } } // namespace Examples diff --git a/src/access/examples/ExampleAccessControlDelegate.h b/src/access/examples/ExampleAccessControlDelegate.h index a6d095905e027c..add53e3114e5d8 100644 --- a/src/access/examples/ExampleAccessControlDelegate.h +++ b/src/access/examples/ExampleAccessControlDelegate.h @@ -30,9 +30,9 @@ namespace Examples { * not manage lifecycle considerations. * * @param storageDelegate Storage instance to access persisted ACL data. - * @return a reference to the AccessControl::Delegate singleton. + * @return a pointer to the AccessControl::Delegate singleton. */ -AccessControl::Delegate & GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate); +AccessControl::Delegate * GetAccessControlDelegate(PersistentStorageDelegate * storageDelegate); } // namespace Examples } // namespace Access diff --git a/src/access/examples/PermissiveAccessControlDelegate.cpp b/src/access/examples/PermissiveAccessControlDelegate.cpp index 3ce22908e2ec9f..ddee48a9f48e48 100644 --- a/src/access/examples/PermissiveAccessControlDelegate.cpp +++ b/src/access/examples/PermissiveAccessControlDelegate.cpp @@ -69,10 +69,10 @@ namespace chip { namespace Access { namespace Examples { -AccessControl::Delegate & GetPermissiveAccessControlDelegate() +AccessControl::Delegate * GetPermissiveAccessControlDelegate() { static AccessControlDelegate accessControlDelegate; - return accessControlDelegate; + return &accessControlDelegate; } } // namespace Examples diff --git a/src/access/examples/PermissiveAccessControlDelegate.h b/src/access/examples/PermissiveAccessControlDelegate.h index bea5f24e616d07..0734e38ff61fe7 100644 --- a/src/access/examples/PermissiveAccessControlDelegate.h +++ b/src/access/examples/PermissiveAccessControlDelegate.h @@ -22,7 +22,7 @@ namespace chip { namespace Access { namespace Examples { -AccessControl::Delegate & GetPermissiveAccessControlDelegate(); +AccessControl::Delegate * GetPermissiveAccessControlDelegate(); } // namespace Examples } // namespace Access diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp index 4b180b91f8e86f..df37578d9745e2 100644 --- a/src/access/tests/TestAccessControl.cpp +++ b/src/access/tests/TestAccessControl.cpp @@ -33,7 +33,7 @@ using Entry = AccessControl::Entry; using EntryIterator = AccessControl::EntryIterator; using Target = Entry::Target; -AccessControl accessControl(Examples::GetAccessControlDelegate(nullptr)); +AccessControl accessControl; constexpr ClusterId kOnOffCluster = 0x0000'0006; constexpr ClusterId kLevelControlCluster = 0x0000'0008; @@ -2133,6 +2133,8 @@ void TestUpdateEntry(nlTestSuite * inSuite, void * inContext) int Setup(void * inContext) { + AccessControl::Delegate *delegate = Examples::GetAccessControlDelegate(nullptr); + accessControl.SetDelegate(delegate); SetAccessControl(accessControl); GetAccessControl().Init(); return SUCCESS; diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 137eb2cf974de2..7e72e7941b5b08 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -107,13 +107,13 @@ Server::Server() : .devicePool = &mDevicePool, .dnsResolver = nullptr, }), - mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage), - mAccessControl(Access::Examples::GetAccessControlDelegate(&mDeviceStorage)) -{} + mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage) {} CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort, Inet::InterfaceId interfaceId) { + Access::AccessControl::Delegate * accessDelegate = nullptr; + mSecuredServicePort = secureServicePort; mUnsecuredServicePort = unsecureServicePort; mInterfaceId = interfaceId; @@ -143,6 +143,10 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint SetGroupDataProvider(&mGroupsProvider); // Access control must be initialized after mDeviceStorage. + accessDelegate = Access::Examples::GetAccessControlDelegate(&mDeviceStorage); + VerifyOrExit(accessDelegate != nullptr, ChipLogError(AppServer, "Invalid access delegate found.")); + mAccessControl.SetDelegate(accessDelegate); + err = mAccessControl.Init(); SuccessOrExit(err); Access::SetAccessControl(mAccessControl); diff --git a/src/app/tests/AppTestContext.cpp b/src/app/tests/AppTestContext.cpp index 817122c02f51ea..416fece48f9413 100644 --- a/src/app/tests/AppTestContext.cpp +++ b/src/app/tests/AppTestContext.cpp @@ -24,7 +24,7 @@ namespace { -chip::Access::AccessControl permissiveAccessControl(chip::Access::Examples::GetPermissiveAccessControlDelegate()); +chip::Access::AccessControl gPermissiveAccessControl; } // namespace @@ -36,7 +36,8 @@ CHIP_ERROR AppContext::Init() ReturnErrorOnFailure(Super::Init()); ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->Init(&GetExchangeManager())); - Access::SetAccessControl(permissiveAccessControl); + gPermissiveAccessControl.SetDelegate(chip::Access::Examples::GetPermissiveAccessControlDelegate()); + Access::SetAccessControl(gPermissiveAccessControl); ReturnErrorOnFailure(Access::GetAccessControl().Init()); return CHIP_NO_ERROR; From 8d2b122a01f7e55341dd63bab395603eebed9ed8 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 10 Mar 2022 17:31:18 +0000 Subject: [PATCH 2/6] Restyled by clang-format --- src/access/AccessControl.cpp | 4 ++-- src/access/AccessControl.h | 2 +- src/access/tests/TestAccessControl.cpp | 2 +- src/app/server/Server.cpp | 3 ++- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index befd33990bf28e..ef02359998619f 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -200,8 +200,8 @@ CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, con "AccessControl: checking f=%u a=%c s=0x" ChipLogFormatX64 " t=%s c=" ChipLogFormatMEI " e=%" PRIu16 " p=%c", subjectDescriptor.fabricIndex, GetAuthModeStringForLogging(subjectDescriptor.authMode), ChipLogValueX64(subjectDescriptor.subject), - GetCatStringForLogging(catLogBuf, sizeof(catLogBuf), subjectDescriptor.cats), ChipLogValueMEI(requestPath.cluster), - requestPath.endpoint, GetPrivilegeStringForLogging(requestPrivilege)); + GetCatStringForLogging(catLogBuf, sizeof(catLogBuf), subjectDescriptor.cats), + ChipLogValueMEI(requestPath.cluster), requestPath.endpoint, GetPrivilegeStringForLogging(requestPrivilege)); } #endif diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h index 8d8cf2889de578..298901110efb3d 100644 --- a/src/access/AccessControl.h +++ b/src/access/AccessControl.h @@ -22,8 +22,8 @@ #include "RequestPath.h" #include "SubjectDescriptor.h" -#include #include "lib/support/CodeUtils.h" +#include namespace chip { namespace Access { diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp index df37578d9745e2..a62b8f661c7dcf 100644 --- a/src/access/tests/TestAccessControl.cpp +++ b/src/access/tests/TestAccessControl.cpp @@ -2133,7 +2133,7 @@ void TestUpdateEntry(nlTestSuite * inSuite, void * inContext) int Setup(void * inContext) { - AccessControl::Delegate *delegate = Examples::GetAccessControlDelegate(nullptr); + AccessControl::Delegate * delegate = Examples::GetAccessControlDelegate(nullptr); accessControl.SetDelegate(delegate); SetAccessControl(accessControl); GetAccessControl().Init(); diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 7e72e7941b5b08..47d233218d21bc 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -107,7 +107,8 @@ Server::Server() : .devicePool = &mDevicePool, .dnsResolver = nullptr, }), - mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage) {} + mCommissioningWindowManager(this), mGroupsProvider(mDeviceStorage), mAttributePersister(mDeviceStorage) +{} CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint16_t unsecureServicePort, Inet::InterfaceId interfaceId) From 614162bddf7d02e2dfa18f18e9c1f070067ae39a Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 10 Mar 2022 13:28:03 -0500 Subject: [PATCH 3/6] Address review comments --- src/access/AccessControl.cpp | 13 +++++---- src/access/AccessControl.h | 37 +++++++------------------- src/access/tests/TestAccessControl.cpp | 3 +-- src/app/server/Server.cpp | 3 +-- src/app/tests/AppTestContext.cpp | 3 +-- 5 files changed, 19 insertions(+), 40 deletions(-) diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index ef02359998619f..91dd30e763d418 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -161,19 +161,18 @@ namespace Access { AccessControl::Entry::Delegate AccessControl::Entry::mDefaultDelegate; AccessControl::EntryIterator::Delegate AccessControl::EntryIterator::mDefaultDelegate; -AccessControl::Delegate AccessControl::mDefaultDelegate; -CHIP_ERROR AccessControl::Init() +CHIP_ERROR AccessControl::Init(AccessControl::Delegate * delegate) { VerifyOrReturnError(!mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); ChipLogProgress(DataManagement, "AccessControl: initializing"); - CHIP_ERROR retval = mDelegate->Init(); - if (retval == CHIP_NO_ERROR) - { - mIsInitialized = true; - } + // delegate can never be null. This was already checked + mDelegate = delegate; + CHIP_ERROR retval = mDelegate->Init(); + mIsInitialized = (CHIP_NO_ERROR == retval); return retval; } diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h index 298901110efb3d..ea63208bd1a09f 100644 --- a/src/access/AccessControl.h +++ b/src/access/AccessControl.h @@ -357,39 +357,23 @@ class AccessControl AccessControl(const AccessControl &) = delete; AccessControl & operator=(const AccessControl &) = delete; - ~AccessControl() { mDelegate->Release(); } - - /** - * @brief Set the delegate that will handle access control entries. Must be called before Init(). - * - * @param delegate - The delegate to use - * @return CHIP_NO_ERROR on success, CHIP_ERROR_INCORRECT_STATE if set after Init(), and - * CHIP_ERROR_INVALID_ARGUMENT if delegate is null. - */ - CHIP_ERROR SetDelegate(Delegate * delegate) - { - if (mIsInitialized) + ~AccessControl() { + // Never-initialized AccessControl instances will not have the delegate set. + if (mIsInitialized && mDelegate != nullptr) { - return CHIP_ERROR_INCORRECT_STATE; - } - else if (delegate == nullptr) - { - return CHIP_ERROR_INVALID_ARGUMENT; + mDelegate->Release(); } - - // De-initialize the default-set delegate. - mDelegate->Release(); - - mDelegate = delegate; - return CHIP_NO_ERROR; } /** * Initialize the access control module. Must be called before first use. * - * @retval various errors, probably fatal. + * @param delegate - The delegate to use for acces control + * + * @return CHIP_NO_ERROR on success, CHIP_ERROR_INCORRECT_STATE if called more than once, + * CHIP_ERROR_INVALID_ARGUMENT if delegate is null, or other fatal error. */ - CHIP_ERROR Init(); + CHIP_ERROR Init(AccessControl::Delegate * delegate); /** * Deinitialize the access control module. Must be called when finished. @@ -501,8 +485,7 @@ class AccessControl private: bool IsValid(const Entry & entry); - static Delegate mDefaultDelegate; - Delegate * mDelegate = &mDefaultDelegate; + Delegate * mDelegate = nullptr; bool mIsInitialized = false; }; diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp index a62b8f661c7dcf..da1000aa41902c 100644 --- a/src/access/tests/TestAccessControl.cpp +++ b/src/access/tests/TestAccessControl.cpp @@ -2134,9 +2134,8 @@ void TestUpdateEntry(nlTestSuite * inSuite, void * inContext) int Setup(void * inContext) { AccessControl::Delegate * delegate = Examples::GetAccessControlDelegate(nullptr); - accessControl.SetDelegate(delegate); SetAccessControl(accessControl); - GetAccessControl().Init(); + VerifyOrDie(GetAccessControl().Init(delegate) == CHIP_NO_ERROR); return SUCCESS; } diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 47d233218d21bc..8cd52093da58a7 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -146,9 +146,8 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint // Access control must be initialized after mDeviceStorage. accessDelegate = Access::Examples::GetAccessControlDelegate(&mDeviceStorage); VerifyOrExit(accessDelegate != nullptr, ChipLogError(AppServer, "Invalid access delegate found.")); - mAccessControl.SetDelegate(accessDelegate); - err = mAccessControl.Init(); + err = mAccessControl.Init(accessDelegate); SuccessOrExit(err); Access::SetAccessControl(mAccessControl); diff --git a/src/app/tests/AppTestContext.cpp b/src/app/tests/AppTestContext.cpp index 416fece48f9413..8fed737f995ff5 100644 --- a/src/app/tests/AppTestContext.cpp +++ b/src/app/tests/AppTestContext.cpp @@ -36,9 +36,8 @@ CHIP_ERROR AppContext::Init() ReturnErrorOnFailure(Super::Init()); ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->Init(&GetExchangeManager())); - gPermissiveAccessControl.SetDelegate(chip::Access::Examples::GetPermissiveAccessControlDelegate()); Access::SetAccessControl(gPermissiveAccessControl); - ReturnErrorOnFailure(Access::GetAccessControl().Init()); + ReturnErrorOnFailure(Access::GetAccessControl().Init(chip::Access::Examples::GetPermissiveAccessControlDelegate())); return CHIP_NO_ERROR; } From 9b0e1a26ec47649e0a341287d10f5a7a447f4680 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 10 Mar 2022 18:28:22 +0000 Subject: [PATCH 4/6] Restyled by clang-format --- src/access/AccessControl.cpp | 4 ++-- src/access/AccessControl.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index 91dd30e763d418..4e037f09c2837f 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -170,9 +170,9 @@ CHIP_ERROR AccessControl::Init(AccessControl::Delegate * delegate) ChipLogProgress(DataManagement, "AccessControl: initializing"); // delegate can never be null. This was already checked - mDelegate = delegate; + mDelegate = delegate; CHIP_ERROR retval = mDelegate->Init(); - mIsInitialized = (CHIP_NO_ERROR == retval); + mIsInitialized = (CHIP_NO_ERROR == retval); return retval; } diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h index ea63208bd1a09f..4aa649c3c5508a 100644 --- a/src/access/AccessControl.h +++ b/src/access/AccessControl.h @@ -357,7 +357,8 @@ class AccessControl AccessControl(const AccessControl &) = delete; AccessControl & operator=(const AccessControl &) = delete; - ~AccessControl() { + ~AccessControl() + { // Never-initialized AccessControl instances will not have the delegate set. if (mIsInitialized && mDelegate != nullptr) { From 40bdcdeb1447ac0823f241480f209f784fbd49ee Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Thu, 10 Mar 2022 14:34:51 -0500 Subject: [PATCH 5/6] Address review comments to remove mIsInitialized --- src/access/AccessControl.cpp | 18 ++++++++++-------- src/access/AccessControl.h | 22 +++++++++++----------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index 4e037f09c2837f..b3ad4ea755ae62 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -164,32 +164,34 @@ AccessControl::EntryIterator::Delegate AccessControl::EntryIterator::mDefaultDel CHIP_ERROR AccessControl::Init(AccessControl::Delegate * delegate) { - VerifyOrReturnError(!mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(!IsInitialized(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); ChipLogProgress(DataManagement, "AccessControl: initializing"); // delegate can never be null. This was already checked - mDelegate = delegate; - CHIP_ERROR retval = mDelegate->Init(); - mIsInitialized = (CHIP_NO_ERROR == retval); + CHIP_ERROR retval = delegate->Init(); + if (retval == CHIP_NO_ERROR) + { + mDelegate = delegate; + } + return retval; } CHIP_ERROR AccessControl::Finish() { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); ChipLogProgress(DataManagement, "AccessControl: finishing"); CHIP_ERROR retval = mDelegate->Finish(); - - mIsInitialized = false; + mDelegate = nullptr; return retval; } CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, const RequestPath & requestPath, Privilege requestPrivilege) { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); #if CHIP_PROGRESS_LOGGING { diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h index 4aa649c3c5508a..ce31345bb0256c 100644 --- a/src/access/AccessControl.h +++ b/src/access/AccessControl.h @@ -360,7 +360,7 @@ class AccessControl ~AccessControl() { // Never-initialized AccessControl instances will not have the delegate set. - if (mIsInitialized && mDelegate != nullptr) + if (IsInitialized()) { mDelegate->Release(); } @@ -384,14 +384,14 @@ class AccessControl // Capabilities CHIP_ERROR GetMaxEntryCount(size_t & value) const { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->GetMaxEntryCount(value); } // Actualities CHIP_ERROR GetEntryCount(size_t & value) const { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->GetEntryCount(value); } @@ -404,7 +404,7 @@ class AccessControl */ CHIP_ERROR PrepareEntry(Entry & entry) { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->PrepareEntry(entry); } @@ -418,7 +418,7 @@ class AccessControl CHIP_ERROR CreateEntry(size_t * index, const Entry & entry, FabricIndex * fabricIndex = nullptr) { ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->CreateEntry(index, entry, fabricIndex); } @@ -431,7 +431,7 @@ class AccessControl */ CHIP_ERROR ReadEntry(size_t index, Entry & entry, const FabricIndex * fabricIndex = nullptr) const { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->ReadEntry(index, entry, fabricIndex); } @@ -445,7 +445,7 @@ class AccessControl CHIP_ERROR UpdateEntry(size_t index, const Entry & entry, const FabricIndex * fabricIndex = nullptr) { ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->UpdateEntry(index, entry, fabricIndex); } @@ -457,7 +457,7 @@ class AccessControl */ CHIP_ERROR DeleteEntry(size_t index, const FabricIndex * fabricIndex = nullptr) { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->DeleteEntry(index, fabricIndex); } @@ -469,7 +469,7 @@ class AccessControl */ CHIP_ERROR Entries(EntryIterator & iterator, const FabricIndex * fabricIndex = nullptr) const { - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); return mDelegate->Entries(iterator, fabricIndex); } @@ -484,11 +484,11 @@ class AccessControl CHIP_ERROR Check(const SubjectDescriptor & subjectDescriptor, const RequestPath & requestPath, Privilege requestPrivilege); private: + bool IsInitialized() const { return (mDelegate != nullptr); } + bool IsValid(const Entry & entry); Delegate * mDelegate = nullptr; - - bool mIsInitialized = false; }; /** From aacd6f53fae8a70d099979f74af94111848e2266 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 10 Mar 2022 19:35:12 +0000 Subject: [PATCH 6/6] Restyled by clang-format --- src/access/AccessControl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index b3ad4ea755ae62..44a880965b74ee 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -184,7 +184,7 @@ CHIP_ERROR AccessControl::Finish() VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE); ChipLogProgress(DataManagement, "AccessControl: finishing"); CHIP_ERROR retval = mDelegate->Finish(); - mDelegate = nullptr; + mDelegate = nullptr; return retval; }