Address feedback from project-chip#22197

sharadb-amazon · Aug 26, 2022 · 04dc37a · 04dc37a
1 parent 5c2334a
commit 04dc37a
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 18 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h b/src/darwin/Framework/CHIP/MTRNOCChainIssuer.h
@@ -22,6 +22,9 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+typedef BOOL (^MTRNOCChainGenerationCompleteHandler)(NSData * operationalCertificate, NSData * intermediateCertificate,
+    NSData * rootCertificate, NSData * _Nullable ipk, NSNumber * _Nullable adminSubject, NSError * __autoreleasing * error);
+
 @protocol MTRNOCChainIssuer <NSObject>
 @required
 
@@ -43,9 +46,7 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)onNOCChainGenerationNeeded:(CSRInfo *)csrInfo
                    attestationInfo:(AttestationInfo *)attestationInfo
-      onNOCChainGenerationComplete:(void (^)(NSData * operationalCertificate, NSData * intermediateCertificate,
-                                       NSData * rootCertificate, NSData * ipk, NSNumber * adminSubject,
-                                       NSError * __autoreleasing * error))onNOCChainGenerationComplete;
+      onNOCChainGenerationComplete:(MTRNOCChainGenerationCompleteHandler)onNOCChainGenerationComplete;
 
 @end
 

diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.h
@@ -127,7 +127,7 @@ class MTROperationalCredentialsDelegate : public chip::Controller::OperationalCr
      * If ipk and adminSubject are non nil, then they will be used in the AddNOC command sent to the commissionee. If they are not
      * populated, then the values provided in the MTRDeviceController initialization will be used.
      */
-    void onNOCChainGenerationComplete(NSData * operationalCertificate, NSData * intermediateCertificate, NSData * rootCertificate,
+    BOOL onNOCChainGenerationComplete(NSData * operationalCertificate, NSData * intermediateCertificate, NSData * rootCertificate,
         NSData * _Nullable ipk, NSNumber * _Nullable adminSubject, NSError * __autoreleasing * error);
 
     void setNSError(CHIP_ERROR err, NSError * __autoreleasing * outError);

diff --git a/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm b/src/darwin/Framework/CHIP/MTROperationalCredentialsDelegate.mm
@@ -178,12 +178,10 @@
     chip::ByteSpan firmwareInfoSpan;
     chip::Credentials::DeviceAttestationVendorReservedDeconstructor vendorReserved;
 
-    __block chip::Optional<chip::Controller::CommissioningParameters> commissioningParameters;
     // Dereferencing mCppCommissioner as it would be set to point to a valid Cpp commissioner by now, as we are in the middle of
     // commissioning
-    dispatch_sync(mChipWorkQueue, ^{
-        commissioningParameters = mCppCommissioner->GetCommissioningParameters();
-    });
+    chip::Optional<chip::Controller::CommissioningParameters> commissioningParameters
+        = mCppCommissioner->GetCommissioningParameters();
     VerifyOrReturnError(commissioningParameters.HasValue(), CHIP_ERROR_INCORRECT_STATE);
 
     // Attestation Elements, nonce and signature will have a value in Commissioning Params as the CSR needs a signature or else we
@@ -205,9 +203,9 @@
     dispatch_sync(mNocChainIssuerQueue, ^{
         [mNocChainIssuer onNOCChainGenerationNeeded:csrInfo
                                     attestationInfo:attestationInfo
-                       onNOCChainGenerationComplete:^void(NSData * operationalCertificate, NSData * intermediateCertificate,
+                       onNOCChainGenerationComplete:^BOOL(NSData * operationalCertificate, NSData * intermediateCertificate,
                            NSData * rootCertificate, NSData * ipk, NSNumber * adminSubject, NSError * __autoreleasing * error) {
-                           onNOCChainGenerationComplete(
+                           return onNOCChainGenerationComplete(
                                operationalCertificate, intermediateCertificate, rootCertificate, ipk, adminSubject, error);
                        }];
     });
@@ -222,25 +220,23 @@
     }
 }
 
-void MTROperationalCredentialsDelegate::onNOCChainGenerationComplete(NSData * operationalCertificate,
+BOOL MTROperationalCredentialsDelegate::onNOCChainGenerationComplete(NSData * operationalCertificate,
     NSData * intermediateCertificate, NSData * rootCertificate, NSData * _Nullable ipk, NSNumber * _Nullable adminSubject,
     NSError * __autoreleasing * error)
 {
     if (operationalCertificate == nil || intermediateCertificate == nil || rootCertificate == nil) {
         setNSError(CHIP_ERROR_INVALID_ARGUMENT, error);
-        return;
+        return NO;
     }
 
     // use ipk and adminSubject from CommissioningParameters if not passed in.
     // Dereferencing mCppCommissioner as it would be set to point to a valid Cpp commissioner by now, as we are in the middle of
     // commissioning
-    __block chip::Optional<chip::Controller::CommissioningParameters> commissioningParameters;
-    dispatch_sync(mChipWorkQueue, ^{
-        commissioningParameters = mCppCommissioner->GetCommissioningParameters();
-    });
+    chip::Optional<chip::Controller::CommissioningParameters> commissioningParameters
+        = mCppCommissioner->GetCommissioningParameters();
     if (!commissioningParameters.HasValue()) {
         setNSError(CHIP_ERROR_INCORRECT_STATE, error);
-        return;
+        return NO;
     }
 
     chip::Optional<chip::Crypto::AesCcm128KeySpan> ipkOptional;
@@ -249,7 +245,7 @@
     if (ipk != nil) {
         if ([ipk length] != sizeof(ipkValue)) {
             setNSError(CHIP_ERROR_INCORRECT_STATE, error);
-            return;
+            return NO;
         }
         memcpy(&ipkValue[0], [ipk bytes], [ipk length]);
         ipkOptional.SetValue(ipkTempSpan);
@@ -273,7 +269,9 @@
     if (err != CHIP_NO_ERROR) {
         MTR_LOG_ERROR("Failed to SetNocChain for the device: %" CHIP_ERROR_FORMAT, err.Format());
         setNSError(CHIP_ERROR_INCORRECT_STATE, error);
+        return NO;
     }
+    return YES;
 }
 
 CHIP_ERROR MTROperationalCredentialsDelegate::LocalGenerateNOCChain(const chip::ByteSpan & csrElements,