From 980e1770075e5209d719693bb2e70d1966e4949f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 25 Feb 2022 03:38:19 -0500 Subject: [PATCH] Stop using __FILE__ in various logging in device code. (#15523) Stops using ReturnErrorOnFailure, LogErrorOnFailure and ReturnLogErrorOnFailure in core code. Converts VerifyOrReturnLogError to not using __FILE__ when CHIP_CONFIG_ERROR_SOURCE is not defined. --- src/app/CASESessionManager.cpp | 12 ++++++++++-- src/app/CommandHandler.cpp | 2 +- src/app/CommandSender.cpp | 6 +++--- .../general-commissioning-server.cpp | 6 +++++- src/app/server/Server.cpp | 12 ++++++++++-- src/lib/support/CodeUtils.h | 12 ++++++++++++ 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/app/CASESessionManager.cpp b/src/app/CASESessionManager.cpp index 7c81100ee0e6f7..8690f036f6353f 100644 --- a/src/app/CASESessionManager.cpp +++ b/src/app/CASESessionManager.cpp @@ -95,14 +95,22 @@ void CASESessionManager::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData if (mConfig.dnsCache != nullptr) { - LogErrorOnFailure(mConfig.dnsCache->Insert(nodeData)); + CHIP_ERROR err = mConfig.dnsCache->Insert(nodeData); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, "DNS Cache insert: %" CHIP_ERROR_FORMAT, err.Format()); + } } OperationalDeviceProxy * session = FindExistingSession(nodeData.mPeerId); VerifyOrReturn(session != nullptr, ChipLogDetail(Controller, "OnNodeIdResolved was called for a device with no active sessions, ignoring it.")); - LogErrorOnFailure(session->UpdateDeviceData(OperationalDeviceProxy::ToPeerAddress(nodeData), nodeData.GetMRPConfig())); + CHIP_ERROR err = session->UpdateDeviceData(OperationalDeviceProxy::ToPeerAddress(nodeData), nodeData.GetMRPConfig()); + if (err != CHIP_NO_ERROR) + { + ChipLogError(Controller, "Update Service Data: %" CHIP_ERROR_FORMAT, err.Format()); + } } void CASESessionManager::OnOperationalNodeResolutionFailed(const PeerId & peer, CHIP_ERROR error) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index e2ed4aa61d368d..945d1384a68f0a 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -434,7 +434,7 @@ CHIP_ERROR CommandHandler::AddStatusInternal(const ConcreteCommandPath & aComman const Optional & aClusterStatus) { StatusIB statusIB; - ReturnLogErrorOnFailure(PrepareStatus(aCommandPath)); + ReturnErrorOnFailure(PrepareStatus(aCommandPath)); CommandStatusIB::Builder & commandStatus = mInvokeResponseBuilder.GetInvokeResponses().GetInvokeResponse().GetStatus(); StatusIB::Builder & statusIBBuilder = commandStatus.CreateErrorStatus(); ReturnErrorOnFailure(commandStatus.GetError()); diff --git a/src/app/CommandSender.cpp b/src/app/CommandSender.cpp index 5739a38240a296..05b183d59211e1 100644 --- a/src/app/CommandSender.cpp +++ b/src/app/CommandSender.cpp @@ -321,7 +321,7 @@ CHIP_ERROR CommandSender::ProcessInvokeResponseIB(InvokeResponseIB::Parser & aIn CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathParams, bool aStartDataStruct) { - ReturnLogErrorOnFailure(AllocateBuffer()); + ReturnErrorOnFailure(AllocateBuffer()); // // We must not be in the middle of preparing a command, or having prepared or sent one. @@ -336,8 +336,8 @@ CHIP_ERROR CommandSender::PrepareCommand(const CommandPathParams & aCommandPathP if (aStartDataStruct) { - ReturnLogErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), - TLV::kTLVType_Structure, mDataElementContainerType)); + ReturnErrorOnFailure(invokeRequest.GetWriter()->StartContainer(TLV::ContextTag(to_underlying(CommandDataIB::Tag::kData)), + TLV::kTLVType_Structure, mDataElementContainerType)); } MoveToState(State::AddingCommand); diff --git a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp index 8dec6607c4fd2c..c64a9ca6f46913 100644 --- a/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp +++ b/src/app/clusters/general-commissioning-server/general-commissioning-server.cpp @@ -45,7 +45,11 @@ using namespace chip::DeviceLayer; { \ if (!::chip::ChipError::IsSuccess(expr)) \ { \ - LogErrorOnFailure(commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::code)); \ + CHIP_ERROR statusErr = commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::code); \ + if (statusErr != CHIP_NO_ERROR) \ + { \ + ChipLogError(Zcl, "%s: %" CHIP_ERROR_FORMAT, #expr, statusErr.Format()); \ + } \ return true; \ } \ } while (false) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 4082c67f23c6d9..f77f97dcaa02c3 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -68,7 +68,11 @@ constexpr bool isRendezvousBypassed() void StopEventLoop(intptr_t arg) { - LogErrorOnFailure(chip::DeviceLayer::PlatformMgr().StopEventLoopTask()); + CHIP_ERROR err = chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + if (err != CHIP_NO_ERROR) + { + ChipLogError(AppServer, "Stopping event loop: %" CHIP_ERROR_FORMAT, err.Format()); + } } void DispatchShutDownEvent(intptr_t arg) @@ -332,7 +336,11 @@ void Server::Shutdown() { chip::Dnssd::ServiceAdvertiser::Instance().Shutdown(); chip::app::InteractionModelEngine::GetInstance()->Shutdown(); - LogErrorOnFailure(mExchangeMgr.Shutdown()); + CHIP_ERROR err = mExchangeMgr.Shutdown(); + if (err != CHIP_NO_ERROR) + { + ChipLogError(AppServer, "Exchange Mgr shutdown: %" CHIP_ERROR_FORMAT, err.Format()); + } mSessions.Shutdown(); mTransports.Close(); mCommissioningWindowManager.Shutdown(); diff --git a/src/lib/support/CodeUtils.h b/src/lib/support/CodeUtils.h index aa06454b9bdc6e..5b4dbbfbdd3515 100644 --- a/src/lib/support/CodeUtils.h +++ b/src/lib/support/CodeUtils.h @@ -309,6 +309,7 @@ constexpr inline const _T & max(const _T & a, const _T & b) * @param[in] expr A Boolean expression to be evaluated. * @param[in] code A value to return if @a expr is false. */ +#if CHIP_CONFIG_ERROR_SOURCE #define VerifyOrReturnLogError(expr, code) \ do \ { \ @@ -318,6 +319,17 @@ constexpr inline const _T & max(const _T & a, const _T & b) return code; \ } \ } while (false) +#else // CHIP_CONFIG_ERROR_SOURCE +#define VerifyOrReturnLogError(expr, code) \ + do \ + { \ + if (!(expr)) \ + { \ + ChipLogError(NotSpecified, "%s:%d false: %" CHIP_ERROR_FORMAT, #expr, __LINE__, code.Format()); \ + return code; \ + } \ + } while (false) +#endif // CHIP_CONFIG_ERROR_SOURCE /** * @def ReturnErrorCodeIf(expr, code)