Use pointer for subject descriptor in datamodel provider instead of std::optional (#36246)
* Use a pointer for the subject descriptor.
This seems to save about 88 bytes of flash on a test NRF board.
* Restyle
* Fix include
* Fix include
* Also fix PW rpc
---------
Co-authored-by: Andrei Litvin <andreilitvin@google.com>
diff --git a/examples/common/pigweed/rpc_services/Attributes.h b/examples/common/pigweed/rpc_services/Attributes.h
index e4ced64..d34d7e5 100644
--- a/examples/common/pigweed/rpc_services/Attributes.h
+++ b/examples/common/pigweed/rpc_services/Attributes.h
@@ -224,7 +224,7 @@
app::DataModel::ReadAttributeRequest request;
request.path = path;
request.operationFlags.Set(app::DataModel::OperationFlags::kInternal);
- request.subjectDescriptor = subjectDescriptor;
+ request.subjectDescriptor = &subjectDescriptor;
std::optional<app::DataModel::ClusterInfo> info = provider->GetClusterInfo(path);
if (!info.has_value())
diff --git a/src/app/CommandHandlerImpl.cpp b/src/app/CommandHandlerImpl.cpp
index 2142af6..80373dc 100644
--- a/src/app/CommandHandlerImpl.cpp
+++ b/src/app/CommandHandlerImpl.cpp
@@ -18,6 +18,7 @@
#include <app/CommandHandlerImpl.h>
#include <access/AccessControl.h>
+#include <access/SubjectDescriptor.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app/MessageDef/StatusIB.h>
#include <app/RequiredPrivilege.h>
@@ -391,10 +392,11 @@
VerifyOrReturnError(err == CHIP_NO_ERROR, Status::InvalidAction);
{
+ Access::SubjectDescriptor subjectDescriptor = GetSubjectDescriptor();
DataModel::InvokeRequest request;
request.path = concretePath;
- request.subjectDescriptor = GetSubjectDescriptor();
+ request.subjectDescriptor = &subjectDescriptor;
request.invokeFlags.Set(DataModel::InvokeFlags::kTimed, IsTimedInvoke());
Status preCheckStatus = mpCallback->ValidateCommandCanBeDispatched(request);
@@ -513,10 +515,11 @@
const ConcreteCommandPath concretePath(mapping.endpoint_id, clusterId, commandId);
{
+ Access::SubjectDescriptor subjectDescriptor = GetSubjectDescriptor();
DataModel::InvokeRequest request;
request.path = concretePath;
- request.subjectDescriptor = GetSubjectDescriptor();
+ request.subjectDescriptor = &subjectDescriptor;
request.invokeFlags.Set(DataModel::InvokeFlags::kTimed, IsTimedInvoke());
Status preCheckStatus = mpCallback->ValidateCommandCanBeDispatched(request);
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
index d88299c..40ec6e7 100644
--- a/src/app/InteractionModelEngine.cpp
+++ b/src/app/InteractionModelEngine.cpp
@@ -1646,10 +1646,12 @@
{
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
+ Access::SubjectDescriptor subjectDescriptor = apCommandObj.GetSubjectDescriptor();
+
DataModel::InvokeRequest request;
request.path = aCommandPath;
request.invokeFlags.Set(DataModel::InvokeFlags::kTimed, apCommandObj.IsTimedInvoke());
- request.subjectDescriptor = apCommandObj.GetSubjectDescriptor();
+ request.subjectDescriptor = &subjectDescriptor;
std::optional<DataModel::ActionReturnStatus> status = GetDataModelProvider()->Invoke(request, apPayload, &apCommandObj);
@@ -1702,7 +1704,7 @@
Protocols::InteractionModel::Status InteractionModelEngine::CheckCommandAccess(const DataModel::InvokeRequest & aRequest)
{
- if (!aRequest.subjectDescriptor.has_value())
+ if (aRequest.subjectDescriptor == nullptr)
{
return Status::UnsupportedAccess; // we require a subject for invoke
}
diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp
index e7787bd..7a1bbc5 100644
--- a/src/app/WriteHandler.cpp
+++ b/src/app/WriteHandler.cpp
@@ -779,7 +779,7 @@
DataModel::WriteAttributeRequest request;
request.path = aPath;
- request.subjectDescriptor = aSubject;
+ request.subjectDescriptor = &aSubject;
request.previousSuccessPath = mLastSuccessfullyWrittenPath;
request.writeFlags.Set(DataModel::WriteFlags::kTimed, IsTimedWrite());
diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp
index ea35356..aa357ce 100644
--- a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp
+++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp
@@ -106,7 +106,7 @@
// ACL check for non-internal requests
if (!request.operationFlags.Has(DataModel::OperationFlags::kInternal))
{
- VerifyOrReturnError(request.subjectDescriptor.has_value(), CHIP_ERROR_INVALID_ARGUMENT);
+ VerifyOrReturnError(request.subjectDescriptor != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
Access::RequestPath requestPath{ .cluster = request.path.mClusterId,
.endpoint = request.path.mEndpointId,
diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp
index 51807fe..de2f888 100644
--- a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp
+++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp
@@ -159,7 +159,7 @@
if (checkAcl)
{
- VerifyOrReturnError(request.subjectDescriptor.has_value(), Status::UnsupportedAccess);
+ VerifyOrReturnError(request.subjectDescriptor != nullptr, Status::UnsupportedAccess);
Access::RequestPath requestPath{ .cluster = request.path.mClusterId,
.endpoint = request.path.mEndpointId,
diff --git a/src/app/data-model-provider/OperationTypes.h b/src/app/data-model-provider/OperationTypes.h
index 6bfffcc..294bf26 100644
--- a/src/app/data-model-provider/OperationTypes.h
+++ b/src/app/data-model-provider/OperationTypes.h
@@ -55,7 +55,7 @@
/// - operationFlags.Has(OperationFlags::kInternal) MUST NOT have this set
///
/// NOTE: once kInternal flag is removed, this will become non-optional
- std::optional<chip::Access::SubjectDescriptor> subjectDescriptor;
+ const chip::Access::SubjectDescriptor * subjectDescriptor = nullptr;
/// Accessing fabric index is the subjectDescriptor fabric index (if any).
/// This is a readability convenience function.
@@ -63,7 +63,8 @@
/// Returns kUndefinedFabricIndex if no subject descriptor is available
FabricIndex GetAccessingFabricIndex() const
{
- return subjectDescriptor.has_value() ? subjectDescriptor->fabricIndex : kUndefinedFabricIndex;
+ VerifyOrReturnValue(subjectDescriptor != nullptr, kUndefinedFabricIndex);
+ return subjectDescriptor->fabricIndex;
}
};
diff --git a/src/app/data-model-provider/tests/ReadTesting.h b/src/app/data-model-provider/tests/ReadTesting.h
index f7cee20..e34a833 100644
--- a/src/app/data-model-provider/tests/ReadTesting.h
+++ b/src/app/data-model-provider/tests/ReadTesting.h
@@ -136,7 +136,7 @@
ReadOperation(const ConcreteAttributePath & path)
{
mRequest.path = path;
- mRequest.subjectDescriptor = kDenySubjectDescriptor;
+ mRequest.subjectDescriptor = &kDenySubjectDescriptor;
}
ReadOperation(EndpointId endpoint, ClusterId cluster, AttributeId attribute) :
@@ -146,7 +146,7 @@
ReadOperation & SetSubjectDescriptor(const chip::Access::SubjectDescriptor & descriptor)
{
VerifyOrDie(mState == State::kInitializing);
- mRequest.subjectDescriptor = descriptor;
+ mRequest.subjectDescriptor = &descriptor;
return *this;
}
diff --git a/src/app/data-model-provider/tests/WriteTesting.h b/src/app/data-model-provider/tests/WriteTesting.h
index d18fb93..7651ffe 100644
--- a/src/app/data-model-provider/tests/WriteTesting.h
+++ b/src/app/data-model-provider/tests/WriteTesting.h
@@ -47,7 +47,7 @@
WriteOperation(const ConcreteDataAttributePath & path)
{
mRequest.path = path;
- mRequest.subjectDescriptor = kDenySubjectDescriptor;
+ mRequest.subjectDescriptor = &kDenySubjectDescriptor;
}
WriteOperation(EndpointId endpoint, ClusterId cluster, AttributeId attribute) :
@@ -56,7 +56,7 @@
WriteOperation & SetSubjectDescriptor(const chip::Access::SubjectDescriptor & descriptor)
{
- mRequest.subjectDescriptor = descriptor;
+ mRequest.subjectDescriptor = &descriptor;
return *this;
}
@@ -123,7 +123,11 @@
AttributeValueDecoder DecoderFor(const T & value)
{
mTLVReader = ReadEncodedValue(value);
- return AttributeValueDecoder(mTLVReader, mRequest.subjectDescriptor.value_or(kDenySubjectDescriptor));
+ if (mRequest.subjectDescriptor == nullptr)
+ {
+ AttributeValueDecoder(mTLVReader, kDenySubjectDescriptor);
+ }
+ return AttributeValueDecoder(mTLVReader, *mRequest.subjectDescriptor);
}
private:
diff --git a/src/app/reporting/Read-DataModel.cpp b/src/app/reporting/Read-DataModel.cpp
index 64d027e..584536b 100644
--- a/src/app/reporting/Read-DataModel.cpp
+++ b/src/app/reporting/Read-DataModel.cpp
@@ -47,7 +47,7 @@
{
readRequest.readFlags.Set(DataModel::ReadFlags::kFabricFiltered);
}
- readRequest.subjectDescriptor = subjectDescriptor;
+ readRequest.subjectDescriptor = &subjectDescriptor;
readRequest.path = path;
DataVersion version = 0;
diff --git a/src/app/tests/test-interaction-model-api.cpp b/src/app/tests/test-interaction-model-api.cpp
index b69c423..33097d3 100644
--- a/src/app/tests/test-interaction-model-api.cpp
+++ b/src/app/tests/test-interaction-model-api.cpp
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+#include "access/SubjectDescriptor.h"
#include <app/tests/test-interaction-model-api.h>
#include <app/InteractionModelEngine.h>
@@ -171,8 +172,13 @@
{
AttributeEncodeState mutableState(&encoder.GetState()); // provide a state copy to start.
- CHIP_ERROR err = ReadSingleClusterData(request.subjectDescriptor.value_or(Access::SubjectDescriptor()),
- request.readFlags.Has(ReadFlags::kFabricFiltered), request.path,
+ Access::SubjectDescriptor subjectDescriptor;
+ if (request.subjectDescriptor != nullptr)
+ {
+ subjectDescriptor = *request.subjectDescriptor;
+ }
+
+ CHIP_ERROR err = ReadSingleClusterData(subjectDescriptor, request.readFlags.Has(ReadFlags::kFabricFiltered), request.path,
TestOnlyAttributeValueEncoderAccessor(encoder).Builder(), &mutableState);
// state must survive CHIP_ERRORs as it is used for chunking
diff --git a/src/controller/tests/data_model/DataModelFixtures.cpp b/src/controller/tests/data_model/DataModelFixtures.cpp
index a5533dc..f007275 100644
--- a/src/controller/tests/data_model/DataModelFixtures.cpp
+++ b/src/controller/tests/data_model/DataModelFixtures.cpp
@@ -18,6 +18,7 @@
#include "DataModelFixtures.h"
+#include <access/SubjectDescriptor.h>
#include <app-common/zap-generated/cluster-objects.h>
#include <app-common/zap-generated/ids/Clusters.h>
#include <app/AttributeValueDecoder.h>
@@ -522,8 +523,13 @@
}
#endif // CHIP_CONFIG_USE_EMBER_DATA_MODEL && CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
- CHIP_ERROR err = ReadSingleClusterData(request.subjectDescriptor.value_or(Access::SubjectDescriptor()),
- request.readFlags.Has(ReadFlags::kFabricFiltered), request.path,
+ Access::SubjectDescriptor subjectDescriptor;
+ if (request.subjectDescriptor != nullptr)
+ {
+ subjectDescriptor = *request.subjectDescriptor;
+ }
+
+ CHIP_ERROR err = ReadSingleClusterData(subjectDescriptor, request.readFlags.Has(ReadFlags::kFabricFiltered), request.path,
TestOnlyAttributeValueEncoderAccessor(encoder).Builder(), &mutableState);
// state must survive CHIP_ERRORs as it is used for chunking
