Use CHIP_ERROR_BUFFER_TOO_SMALL, Avoid SuccessOrExit, args for example

Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Jul 5, 2024 · d29e4fc · d29e4fc
1 parent 5d4ce20
commit d29e4fc
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 45 deletions.
diff --git a/examples/fabric-bridge-app/linux/RpcClient.cpp b/examples/fabric-bridge-app/linux/RpcClient.cpp
@@ -63,10 +63,9 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort)
     return rpc::client::StartPacketProcessing();
 }
 
-CHIP_ERROR OpenCommissioningWindow(NodeId nodeId, uint16_t commissioningTimeout, uint16_t discriminator, uint32_t iterations,
-                                   chip::Optional<chip::ByteSpan> salt, chip::Optional<chip::ByteSpan> verifier)
+CHIP_ERROR OpenCommissioningWindow(CommissioningWindowParams params)
 {
-    ChipLogProgress(NotSpecified, "OpenCommissioningWindow with Node Id 0x:" ChipLogFormatX64, ChipLogValueX64(nodeId));
+    ChipLogProgress(NotSpecified, "OpenCommissioningWindow with Node Id 0x:" ChipLogFormatX64, ChipLogValueX64(params.nodeId));
 
     if (openCommissioningWindowCall.active())
     {
@@ -75,29 +74,23 @@ CHIP_ERROR OpenCommissioningWindow(NodeId nodeId, uint16_t commissioningTimeout,
     }
 
     chip_rpc_DeviceCommissioningWindowInfo device;
-    device.node_id               = nodeId;
-    device.commissioning_timeout = commissioningTimeout;
-    device.discriminator         = discriminator;
-    device.iterations            = iterations;
+    device.node_id               = params.nodeId;
+    device.commissioning_timeout = params.commissioningTimeout;
+    device.discriminator         = params.discriminator;
+    device.iterations            = params.iterations;
 
-    if (salt.HasValue())
+    if (params.salt.HasValue())
     {
-        if (salt.Value().size() > sizeof(device.salt.bytes))
-        {
-            return CHIP_ERROR_INTERNAL;
-        }
-        memcpy(device.salt.bytes, salt.Value().data(), salt.Value().size());
-        device.salt.size = static_cast<size_t>(salt.Value().size());
+        VerifyOrReturnError(params.salt.Value().size() <= sizeof(device.salt.bytes), CHIP_ERROR_BUFFER_TOO_SMALL);
+        memcpy(device.salt.bytes, params.salt.Value().data(), params.salt.Value().size());
+        device.salt.size = static_cast<size_t>(params.salt.Value().size());
     }
 
-    if (verifier.HasValue())
+    if (params.verifier.HasValue())
     {
-        if (verifier.Value().size() > sizeof(device.verifier.bytes))
-        {
-            return CHIP_ERROR_INTERNAL;
-        }
-        memcpy(device.verifier.bytes, verifier.Value().data(), verifier.Value().size());
-        device.verifier.size = static_cast<size_t>(verifier.Value().size());
+        VerifyOrReturnError(params.verifier.Value().size() <= sizeof(device.verifier.bytes), CHIP_ERROR_BUFFER_TOO_SMALL);
+        memcpy(device.verifier.bytes, params.verifier.Value().data(), params.verifier.Value().size());
+        device.verifier.size = static_cast<size_t>(params.verifier.Value().size());
     }
 
     // The RPC will remain active as long as `openCommissioningWindowCall` is alive.

diff --git a/examples/fabric-bridge-app/linux/include/RpcClient.h b/examples/fabric-bridge-app/linux/include/RpcClient.h
@@ -32,14 +32,24 @@ constexpr uint16_t kFabricAdminServerPort = 33001;
  */
 CHIP_ERROR InitRpcClient(uint16_t rpcServerPort);
 
+struct CommissioningWindowParams
+{
+    chip::NodeId nodeId;
+    uint16_t commissioningTimeout;
+    uint16_t discriminator;
+    uint32_t iterations;
+    chip::Optional<chip::ByteSpan> salt     = chip::NullOptional;
+    chip::Optional<chip::ByteSpan> verifier = chip::NullOptional;
+};
+
 /**
  * Opens a commissioning window for a specified node.
  *
- * @param nodeId The identifier of the node for which the commissioning window should be opened.
+ * @param params    Params for opening the commissioning window on given node.
  * @return CHIP_ERROR An error code indicating the success or failure of the operation.
  * - CHIP_NO_ERROR: The RPC command was successfully sent.
  * - CHIP_ERROR_BUSY: Another commissioning window is currently in progress.
  * - CHIP_ERROR_INTERNAL: An internal error occurred.
  */
-CHIP_ERROR OpenCommissioningWindow(chip::NodeId nodeId, uint16_t commissioningTimeout, uint16_t discriminator, uint32_t iterations,
-                                   chip::Optional<chip::ByteSpan> salt, chip::Optional<chip::ByteSpan> verifier);
+CHIP_ERROR
+OpenCommissioningWindow(CommissioningWindowParams params);
diff --git a/examples/fabric-bridge-app/linux/main.cpp b/examples/fabric-bridge-app/linux/main.cpp
@@ -74,7 +74,10 @@ void BridgePollingThread()
 #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
             else if (ch == 'o')
             {
-                CHIP_ERROR err = OpenCommissioningWindow(0x1234, 300, 3840, 1000, chip::NullOptional, chip::NullOptional);
+                CommissioningWindowParams params = {
+                    .nodeId = 0x1234, .commissioningTimeout = 300, .discriminator = 3840, .iterations = 1000
+                };
+                CHIP_ERROR err = OpenCommissioningWindow(params);
                 if (err != CHIP_NO_ERROR)
                 {
                     ChipLogError(NotSpecified, "Failed to call OpenCommissioningWindow RPC: %" CHIP_ERROR_FORMAT, err.Format());
@@ -130,31 +133,27 @@ void AdministratorCommissioningCommandHandler::InvokeCommand(HandlerContext & ha
 
     handlerContext.SetCommandHandled();
 
-    uint16_t commissioningTimeout;
-    uint16_t discriminator;
-    uint32_t iterations;
-    chip::Optional<chip::ByteSpan> salt;
-    chip::Optional<chip::ByteSpan> verifier;
-    Device * device = nullptr;
-    Status status   = Status::Failure;
-
     Commands::OpenCommissioningWindow::DecodableType commandData;
-    CHIP_ERROR tlvError = DataModel::Decode(handlerContext.mPayload, commandData);
-    SuccessOrExit(tlvError);
+    if (DataModel::Decode(handlerContext.mPayload, commandData) != CHIP_NO_ERROR)
+    {
+        handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, Status::InvalidCommand);
+        return;
+    }
 
-    commissioningTimeout = commandData.commissioningTimeout;
-    discriminator        = commandData.discriminator;
-    iterations           = commandData.iterations;
-    salt                 = chip::Optional<chip::ByteSpan>(commandData.salt);
-    verifier             = chip::Optional<chip::ByteSpan>(commandData.PAKEPasscodeVerifier);
+    Status status = Status::Failure;
 
 #if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
-    device = DeviceMgr().GetDevice(endpointId);
+    Device * device = DeviceMgr().GetDevice(endpointId);
+
+    CommissioningWindowParams params = { .nodeId               = device->GetNodeId(),
+                                         .commissioningTimeout = commandData.commissioningTimeout,
+                                         .discriminator        = commandData.discriminator,
+                                         .iterations           = commandData.iterations,
+                                         .salt                 = chip::Optional<chip::ByteSpan>(commandData.salt),
+                                         .verifier             = chip::Optional<chip::ByteSpan>(commandData.PAKEPasscodeVerifier) };
 
     // TODO: issues:#33784, need to make OpenCommissioningWindow synchronous
-    if (device != nullptr &&
-        OpenCommissioningWindow(device->GetNodeId(), commissioningTimeout, discriminator, iterations, salt, verifier) ==
-            CHIP_NO_ERROR)
+    if (device != nullptr && OpenCommissioningWindow(params) == CHIP_NO_ERROR)
     {
         ChipLogProgress(NotSpecified, "Commissioning window is now open");
         status = Status::Success;
@@ -167,7 +166,6 @@ void AdministratorCommissioningCommandHandler::InvokeCommand(HandlerContext & ha
     ChipLogProgress(NotSpecified, "Commissioning window failed to open: PW_RPC_FABRIC_BRIDGE_SERVICE not defined");
 #endif // defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
 
-exit:
     handlerContext.mCommandHandler.AddStatus(handlerContext.mRequestPath, status);
 }