From a2a0dd49a556e239ed61bba2cb1a962f218c9a9a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 29 Jul 2022 15:58:38 -0400 Subject: [PATCH] Improve error logging during Darwin controller startup. (#21360) Right now we are not reporting any details about what error we encountered. We should report those details. --- .../Framework/CHIP/MTRDeviceController.mm | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index 5a870de5a2ff3d..e8ac978f7bb026 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -196,19 +196,20 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams chip::Crypto::P256Keypair * signingKeypair = nullptr; if (startupParams.nocSigner) { errorCode = _signingKeypairBridge.Init(startupParams.nocSigner); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorSigningKeypairInit]) { + if ([self checkForStartError:errorCode logMsg:kErrorSigningKeypairInit]) { return; } signingKeypair = &_signingKeypairBridge; } errorCode = _operationalCredentialsDelegate->Init(_factory.storageDelegateBridge, signingKeypair, startupParams.ipk, startupParams.rootCertificate, startupParams.intermediateCertificate); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalCredentialsInit]) { + if ([self checkForStartError:errorCode logMsg:kErrorOperationalCredentialsInit]) { return; } _cppCommissioner = new chip::Controller::DeviceCommissioner(); - if ([self checkForStartError:(_cppCommissioner != nullptr) logMsg:kErrorCommissionerInit]) { + if (_cppCommissioner == nullptr) { + [self checkForStartError:CHIP_ERROR_NO_MEMORY logMsg:kErrorCommissionerInit]; return; } @@ -228,7 +229,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams if (startupParams.operationalKeypair != nil) { errorCode = _operationalKeypairBridge.Init(startupParams.operationalKeypair); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalKeypairInit]) { + if ([self checkForStartError:errorCode logMsg:kErrorOperationalKeypairInit]) { return; } commissionerParams.operationalKeypair = &_operationalKeypairBridge; @@ -244,7 +245,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams errorCode = _operationalCredentialsDelegate->GenerateNOC([startupParams.nodeId unsignedLongLongValue], startupParams.fabricId, chip::kUndefinedCATs, commissionerParams.operationalKeypair->Pubkey(), noc); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorGenerateNOC]) { + if ([self checkForStartError:errorCode logMsg:kErrorGenerateNOC]) { return; } } else { @@ -252,20 +253,20 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams uint8_t csrBuffer[chip::Crypto::kMAX_CSR_Length]; chip::MutableByteSpan csr(csrBuffer); errorCode = startupParams.fabricTable->AllocatePendingOperationalKey(startupParams.fabricIndex, csr); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorKeyAllocation]) { + if ([self checkForStartError:errorCode logMsg:kErrorKeyAllocation]) { return; } chip::Crypto::P256PublicKey pubKey; errorCode = VerifyCertificateSigningRequest(csr.data(), csr.size(), pubKey); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCSRValidation]) { + if ([self checkForStartError:errorCode logMsg:kErrorCSRValidation]) { return; } errorCode = _operationalCredentialsDelegate->GenerateNOC( [startupParams.nodeId unsignedLongLongValue], startupParams.fabricId, chip::kUndefinedCATs, pubKey, noc); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorGenerateNOC]) { + if ([self checkForStartError:errorCode logMsg:kErrorGenerateNOC]) { return; } } @@ -277,7 +278,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams auto & factory = chip::Controller::DeviceControllerFactory::GetInstance(); errorCode = factory.SetupCommissioner(commissionerParams, *_cppCommissioner); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) { + if ([self checkForStartError:errorCode logMsg:kErrorCommissionerInit]) { return; } @@ -286,13 +287,13 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams uint8_t compressedIdBuffer[sizeof(uint64_t)]; chip::MutableByteSpan compressedId(compressedIdBuffer); errorCode = _cppCommissioner->GetCompressedFabricIdBytes(compressedId); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorIPKInit]) { + if ([self checkForStartError:errorCode logMsg:kErrorIPKInit]) { return; } errorCode = chip::Credentials::SetSingleIpkEpochKey( _factory.groupData, fabricIdx, _operationalCredentialsDelegate->GetIPK(), compressedId); - if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorIPKInit]) { + if ([self checkForStartError:errorCode logMsg:kErrorIPKInit]) { return; } @@ -637,13 +638,13 @@ - (void)clearDeviceAttestationDelegateBridge } } -- (BOOL)checkForStartError:(BOOL)condition logMsg:(NSString *)logMsg +- (BOOL)checkForStartError:(CHIP_ERROR)errorCode logMsg:(NSString *)logMsg { - if (condition) { + if (CHIP_NO_ERROR == errorCode) { return NO; } - MTR_LOG_ERROR("Error: %@", logMsg); + MTR_LOG_ERROR("Error(%" CHIP_ERROR_FORMAT "): %@", errorCode.Format(), logMsg); return YES; } @@ -654,7 +655,7 @@ - (BOOL)checkForError:(CHIP_ERROR)errorCode logMsg:(NSString *)logMsg error:(NSE return NO; } - MTR_LOG_ERROR("Error(%s): %s", chip::ErrorStr(errorCode), [logMsg UTF8String]); + MTR_LOG_ERROR("Error(%" CHIP_ERROR_FORMAT "): %s", errorCode.Format(), [logMsg UTF8String]); if (error) { *error = [MTRError errorForCHIPErrorCode:errorCode]; }