From 8fbcd220647bf56456efc6de014ffc3a32370eff Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Sat, 19 Sep 2020 09:05:08 -0700 Subject: [PATCH] [meshcop] add helper function 'LogError()' (#5509) --- src/core/meshcop/border_agent.cpp | 50 ++++++++++---------------- src/core/meshcop/commissioner.cpp | 32 +++++------------ src/core/meshcop/joiner.cpp | 15 ++------ src/core/meshcop/joiner_router.cpp | 12 +++---- src/core/meshcop/meshcop.cpp | 10 ++++++ src/core/meshcop/meshcop.hpp | 18 ++++++++++ src/core/meshcop/meshcop_leader.cpp | 33 +++++++---------- src/core/thread/energy_scan_server.cpp | 11 +++--- src/core/thread/panid_query_server.cpp | 11 +++--- 9 files changed, 83 insertions(+), 109 deletions(-) diff --git a/src/core/meshcop/border_agent.cpp b/src/core/meshcop/border_agent.cpp index a43c84ad4..59167a59c 100644 --- a/src/core/meshcop/border_agent.cpp +++ b/src/core/meshcop/border_agent.cpp @@ -174,15 +174,12 @@ static void SendErrorMessage(Coap::CoapSecure &aCoapSecure, ForwardContext &aFor SuccessOrExit(error = aCoapSecure.SendMessage(*message, aCoapSecure.GetMessageInfo())); exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - if (message != nullptr) - { - message->Free(); - } - - otLogWarnMeshCoP("Failed to send error CoAP message: %s", otThreadErrorToString(error)); + message->Free(); } + + LogError("send error CoAP message", error); } static void SendErrorMessage(Coap::CoapSecure & aCoapSecure, @@ -214,15 +211,12 @@ static void SendErrorMessage(Coap::CoapSecure & aCoapSecure, SuccessOrExit(error = aCoapSecure.SendMessage(*message, aCoapSecure.GetMessageInfo())); exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - if (message != nullptr) - { - message->Free(); - } - - otLogWarnMeshCoP("Failed to send error CoAP message: %s", otThreadErrorToString(error)); + message->Free(); } + + LogError("send error CoAP message", error); } void BorderAgent::HandleCoapResponse(void * aContext, @@ -407,15 +401,12 @@ void BorderAgent::HandleProxyTransmit(const Coap::Message &aMessage) otLogInfoMeshCoP("Proxy transmit sent"); exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogWarnMeshCoP("Failed to send proxy stream: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + LogError("send proxy stream", error); } bool BorderAgent::HandleUdpReceive(const Message &aMessage, const Ip6::MessageInfo &aMessageInfo) @@ -460,10 +451,11 @@ bool BorderAgent::HandleUdpReceive(const Message &aMessage, const Ip6::MessageIn exit: if (message != nullptr && error != OT_ERROR_NONE) { - otLogWarnMeshCoP("Failed notify commissioner on %s", UriPath::kProxyRx); message->Free(); } + LogError("notify commissioner on ProxyRx (c/ur)", error); + return error != OT_ERROR_DESTINATION_ADDRESS_FILTERED; } @@ -508,11 +500,7 @@ otError BorderAgent::ForwardToCommissioner(Coap::Message &aForwardMessage, const otLogInfoMeshCoP("Sent to commissioner"); exit: - if (error != OT_ERROR_NONE) - { - otLogWarnMeshCoP("Failed to send to commissioner: %s", otThreadErrorToString(error)); - } - + LogError("send to commissioner", error); return error; } @@ -562,13 +550,13 @@ void BorderAgent::HandleRelayTransmit(const Coap::Message &aMessage) exit: if (error != OT_ERROR_NONE) { - otLogWarnMeshCoP("Failed to sent to joiner router request %s: %s", UriPath::kRelayTx, - otThreadErrorToString(error)); if (message != nullptr) { message->Free(); } } + + LogError("send to joiner router request RelayTx (c/tx)", error); } otError BorderAgent::ForwardToLeader(const Coap::Message & aMessage, @@ -620,6 +608,8 @@ otError BorderAgent::ForwardToLeader(const Coap::Message & aMessage, otLogInfoMeshCoP("Forwarded request to leader on %s", aPath); exit: + LogError("forward to leader", error); + if (error != OT_ERROR_NONE) { if (forwardContext != nullptr) @@ -632,8 +622,6 @@ exit: message->Free(); } - otLogWarnMeshCoP("Failed to forward to leader: %s", otThreadErrorToString(error)); - SendErrorMessage(Get(), aMessage, aSeparate, error); } diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index 2400f3fc6..51e292e92 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -321,15 +321,12 @@ otError Commissioner::Start(otCommissionerStateCallback aStateCallback, SetState(kStatePetition); exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (error != OT_ERROR_ALREADY)) { - otLogWarnMeshCoP("Failed to start commissioner: %s", otThreadErrorToString(error)); - if (error != OT_ERROR_ALREADY) - { - Get().Stop(); - } + Get().Stop(); } + LogError("start commissioner", error); return error; } @@ -364,11 +361,7 @@ otError Commissioner::Stop(bool aResign) } exit: - if (error != OT_ERROR_NONE) - { - otLogWarnMeshCoP("Failed to stop Commissioner: %s", otThreadErrorToString(error)); - } - + LogError("stop commissioner", error); return error; } @@ -422,10 +415,7 @@ void Commissioner::SendCommissionerSet(void) error = SendMgmtCommissionerSetRequest(dataset, nullptr, 0); exit: - if (error != OT_ERROR_NONE) - { - otLogWarnMeshCoP("Failed to send MGMT_COMMISSIONER_SET.req: %s", otThreadErrorToString(error)); - } + LogError("send MGMT_COMMISSIONER_SET.req", error); } void Commissioner::ClearJoiners(void) @@ -969,16 +959,12 @@ void Commissioner::SendKeepAlive(uint16_t aSessionId) otLogInfoMeshCoP("sent keep alive"); exit: - - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogWarnMeshCoP("Failed to send keep alive: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + LogError("send keep alive", error); } void Commissioner::HandleLeaderKeepAliveResponse(void * aContext, diff --git a/src/core/meshcop/joiner.cpp b/src/core/meshcop/joiner.cpp index 8b1f755bf..26d024cd2 100644 --- a/src/core/meshcop/joiner.cpp +++ b/src/core/meshcop/joiner.cpp @@ -182,10 +182,10 @@ otError Joiner::Start(const char * aPskd, exit: if (error != OT_ERROR_NONE) { - otLogWarnMeshCoP("Failed to start joiner: %s", otThreadErrorToString(error)); FreeJoinerFinalizeMessage(); } + LogError("start joiner", error); return error; } @@ -393,12 +393,7 @@ otError Joiner::Connect(JoinerRouter &aRouter) SetState(kStateConnect); exit: - - if (error != OT_ERROR_NONE) - { - otLogWarnMeshCoP("Failed to start secure joiner connection: %s", otThreadErrorToString(error)); - } - + LogError("start secure joiner connection", error); return error; } @@ -594,11 +589,7 @@ void Joiner::HandleJoinerEntrust(Coap::Message &aMessage, const Ip6::MessageInfo mTimer.Start(kConfigExtAddressDelay); exit: - - if (error != OT_ERROR_NONE) - { - otLogWarnMeshCoP("Failed to process joiner entrust: %s", otThreadErrorToString(error)); - } + LogError("process joiner entrust", error); } void Joiner::SendJoinerEntrustResponse(const Coap::Message &aRequest, const Ip6::MessageInfo &aRequestInfo) diff --git a/src/core/meshcop/joiner_router.cpp b/src/core/meshcop/joiner_router.cpp index 281d58ad4..204418ee4 100644 --- a/src/core/meshcop/joiner_router.cpp +++ b/src/core/meshcop/joiner_router.cpp @@ -253,16 +253,12 @@ void JoinerRouter::DelaySendingJoinerEntrust(const Ip6::MessageInfo &aMessageInf } exit: - - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogNoteMeshCoP("Failed to schedule joiner entrust: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + LogError("schedule joiner entrust", error); } void JoinerRouter::HandleTimer(Timer &aTimer) diff --git a/src/core/meshcop/meshcop.cpp b/src/core/meshcop/meshcop.cpp index 7de2b94d2..d5fc1b801 100644 --- a/src/core/meshcop/meshcop.cpp +++ b/src/core/meshcop/meshcop.cpp @@ -350,5 +350,15 @@ exit: } #endif // OPENTHREAD_FTD +#if (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_WARN) && (OPENTHREAD_CONFIG_LOG_MESHCOP == 1) +void LogError(const char *aActionText, otError aError) +{ + if (aError != OT_ERROR_NONE) + { + otLogWarnMeshCoP("Failed to %s: %s", aActionText, otThreadErrorToString(aError)); + } +} +#endif + } // namespace MeshCoP } // namespace ot diff --git a/src/core/meshcop/meshcop.hpp b/src/core/meshcop/meshcop.hpp index 1c635001f..ac053de0d 100644 --- a/src/core/meshcop/meshcop.hpp +++ b/src/core/meshcop/meshcop.hpp @@ -493,6 +493,24 @@ void ComputeJoinerId(const Mac::ExtAddress &aEui64, Mac::ExtAddress &aJoinerId); */ otError GetBorderAgentRloc(ThreadNetif &aNetIf, uint16_t &aRloc); +#if (OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_WARN) && (OPENTHREAD_CONFIG_LOG_MESHCOP == 1) +/** + * This function emits a log message indicating an error during a MeshCoP action. + * + * Note that log message is emitted only if there is an error, i.e. @p aError is not `OT_ERROR_NONE`. The log + * message will have the format "Failed to {aActionText} : {ErrorString}". + * + * @param[in] aActionText A string representing the failed action. + * @param[in] aError The error in sending the message. + * + */ +void LogError(const char *aActionText, otError aError); +#else +inline void LogError(const char *, otError) +{ +} +#endif + } // namespace MeshCoP } // namespace ot diff --git a/src/core/meshcop/meshcop_leader.cpp b/src/core/meshcop/meshcop_leader.cpp index 113d5b9c6..888be1208 100644 --- a/src/core/meshcop/meshcop_leader.cpp +++ b/src/core/meshcop/meshcop_leader.cpp @@ -150,15 +150,12 @@ void Leader::SendPetitionResponse(const Coap::Message & aRequest, exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogInfoMeshCoP("Failed to send petition response: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + LogError("send petition response", error); } void Leader::HandleKeepAlive(void *aContext, otMessage *aMessage, const otMessageInfo *aMessageInfo) @@ -232,15 +229,12 @@ void Leader::SendKeepAliveResponse(const Coap::Message & aRequest, exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogWarnMeshCoP("Failed to send keep alive response: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + LogError("send keep alive response", error); } void Leader::SendDatasetChanged(const Ip6::Address &aAddress) @@ -262,15 +256,12 @@ void Leader::SendDatasetChanged(const Ip6::Address &aAddress) exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogWarnMeshCoP("Failed to send dataset changed: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + LogError("send dataset changed", error); } otError Leader::SetDelayTimerMinimal(uint32_t aDelayTimerMinimal) diff --git a/src/core/thread/energy_scan_server.cpp b/src/core/thread/energy_scan_server.cpp index 462983553..7aa2ed679 100644 --- a/src/core/thread/energy_scan_server.cpp +++ b/src/core/thread/energy_scan_server.cpp @@ -198,16 +198,13 @@ void EnergyScanServer::SendReport(void) exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogInfoMeshCoP("Failed to send scan results: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + MeshCoP::LogError("send scan results", error); + mActive = false; } diff --git a/src/core/thread/panid_query_server.cpp b/src/core/thread/panid_query_server.cpp index 5793ffa88..ba9adb3bc 100644 --- a/src/core/thread/panid_query_server.cpp +++ b/src/core/thread/panid_query_server.cpp @@ -135,15 +135,12 @@ void PanIdQueryServer::SendConflict(void) exit: - if (error != OT_ERROR_NONE) + if ((error != OT_ERROR_NONE) && (message != nullptr)) { - otLogWarnMeshCoP("Failed to send panid conflict: %s", otThreadErrorToString(error)); - - if (message != nullptr) - { - message->Free(); - } + message->Free(); } + + MeshCoP::LogError("send panid conflict", error); } void PanIdQueryServer::HandleTimer(Timer &aTimer)