From d71742049f79965797ecab24a7038b8551966b7b Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Thu, 27 Jan 2022 10:29:09 -0800 Subject: [PATCH] [logging] remove `otLogResult` and move to module (#7360) This commit removes the `otLogResult{Bbr/Platform}()` macros from `logging.hpp` and instead have each module implement `LogError()` method. The main goal behind this change to simplify the logging module and reduce number of macros/definitions in it (to prepare for new logging model to be introduced). --- src/core/backbone_router/backbone_tmf.cpp | 25 ++++++++---- src/core/backbone_router/backbone_tmf.hpp | 1 + src/core/backbone_router/bbr_manager.cpp | 16 +++++++- src/core/backbone_router/bbr_manager.hpp | 2 + src/core/common/logging.hpp | 41 ------------------- src/posix/platform/multicast_routing.cpp | 48 +++++++++++++++-------- 6 files changed, 68 insertions(+), 65 deletions(-) diff --git a/src/core/backbone_router/backbone_tmf.cpp b/src/core/backbone_router/backbone_tmf.cpp index cde9d4696..676906f81 100644 --- a/src/core/backbone_router/backbone_tmf.cpp +++ b/src/core/backbone_router/backbone_tmf.cpp @@ -77,20 +77,31 @@ bool BackboneTmfAgent::IsBackboneTmfMessage(const Ip6::MessageInfo &aMessageInfo void BackboneTmfAgent::SubscribeMulticast(const Ip6::Address &aAddress) { - Error error; + Error error = mSocket.JoinNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); - error = mSocket.JoinNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); - - otLogResultBbr(error, "Backbone TMF subscribes %s", aAddress.ToString().AsCString()); + LogError("Backbone TMF subscribes", aAddress, error); } void BackboneTmfAgent::UnsubscribeMulticast(const Ip6::Address &aAddress) { - Error error; + Error error = mSocket.LeaveNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); - error = mSocket.LeaveNetifMulticastGroup(OT_NETIF_BACKBONE, aAddress); + LogError("Backbone TMF unsubscribes", aAddress, error); +} - otLogResultBbr(error, "Backbone TMF unsubscribes %s", aAddress.ToString().AsCString()); +void BackboneTmfAgent::LogError(const char *aText, const Ip6::Address &aAddress, Error aError) const +{ + OT_UNUSED_VARIABLE(aText); + OT_UNUSED_VARIABLE(aAddress); + + if (aError == kErrorNone) + { + otLogInfoBbr("%s %s: %s", aText, aAddress.ToString().AsCString(), ErrorToString(aError)); + } + else + { + otLogWarnBbr("%s %s: %s", aText, aAddress.ToString().AsCString(), ErrorToString(aError)); + } } } // namespace BackboneRouter diff --git a/src/core/backbone_router/backbone_tmf.hpp b/src/core/backbone_router/backbone_tmf.hpp index 31d6ce6f9..0720f05b5 100644 --- a/src/core/backbone_router/backbone_tmf.hpp +++ b/src/core/backbone_router/backbone_tmf.hpp @@ -99,6 +99,7 @@ public: void UnsubscribeMulticast(const Ip6::Address &aAddress); private: + void LogError(const char *aText, const Ip6::Address &aAddress, Error aError) const; static Error Filter(const ot::Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo, void *aContext); }; diff --git a/src/core/backbone_router/bbr_manager.cpp b/src/core/backbone_router/bbr_manager.cpp index 397592244..405d12e7f 100644 --- a/src/core/backbone_router/bbr_manager.cpp +++ b/src/core/backbone_router/bbr_manager.cpp @@ -130,7 +130,7 @@ void Manager::HandleNotifierEvents(Events aEvents) error = mBackboneTmfAgent.Start(); - otLogResultBbr(error, "Start Backbone TMF agent"); + LogError("Start Backbone TMF agent", error); } } } @@ -793,6 +793,20 @@ exit: } #endif // OPENTHREAD_CONFIG_BACKBONE_ROUTER_DUA_NDPROXYING_ENABLE +void Manager::LogError(const char *aText, Error aError) const +{ + OT_UNUSED_VARIABLE(aText); + + if (aError == kErrorNone) + { + otLogInfoBbr("%s: %s", aText, ErrorToString(aError)); + } + else + { + otLogWarnBbr("%s: %s", aText, ErrorToString(aError)); + } +} + } // namespace BackboneRouter } // namespace ot diff --git a/src/core/backbone_router/bbr_manager.hpp b/src/core/backbone_router/bbr_manager.hpp index 2835f7b86..e5f170568 100644 --- a/src/core/backbone_router/bbr_manager.hpp +++ b/src/core/backbone_router/bbr_manager.hpp @@ -229,6 +229,8 @@ private: static void HandleTimer(Timer &aTimer); void HandleTimer(void); + void LogError(const char *aText, Error aError) const; + #if OPENTHREAD_CONFIG_BACKBONE_ROUTER_MULTICAST_ROUTING_ENABLE Coap::Resource mMulticastListenerRegistration; #endif diff --git a/src/core/common/logging.hpp b/src/core/common/logging.hpp index ea9b69881..dc3167b28 100644 --- a/src/core/common/logging.hpp +++ b/src/core/common/logging.hpp @@ -2582,47 +2582,6 @@ void otDumpMacFrame(otLogLevel aLogLevel, const char *aId, const void *aBuf, siz */ void otDump(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aId, const void *aBuf, size_t aLength); -#define _otLogResult(aRegion, aError, ...) \ - do \ - { \ - otError _err = (aError); \ - \ - if (_err == OT_ERROR_NONE) \ - { \ - otLogInfo##aRegion(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), \ - otThreadErrorToString(_err)); \ - } \ - else \ - { \ - otLogWarn##aRegion(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), \ - otThreadErrorToString(_err)); \ - } \ - } while (false) - -/** - * @def otLogResultPlat - * - * This function generates a log for the Plat region according to the error result. If @p aError is `kErrorNone`, the - * log level is info. Otherwise the log level is warn. - * - * @param[in] aError The error result. - * @param[in] ... Arguments for the format specification. - * - */ -#define otLogResultPlat(aError, ...) _otLogResult(Plat, aError, __VA_ARGS__) - -/** - * @def otLogResultBbr - * - * This function generates a log for the BBR region according to the error result. If @p aError is `OT_ERROR_NONE`, the - * log level is info. Otherwise the log level is warn. - * - * @param[in] aError The error result. - * @param[in] ... Arguments for the format specification. - * - */ -#define otLogResultBbr(aError, ...) _otLogResult(Bbr, aError, __VA_ARGS__) - #ifdef __cplusplus } #endif diff --git a/src/posix/platform/multicast_routing.cpp b/src/posix/platform/multicast_routing.cpp index e637ddb69..ab35d03ab 100644 --- a/src/posix/platform/multicast_routing.cpp +++ b/src/posix/platform/multicast_routing.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -52,6 +53,21 @@ namespace ot { namespace Posix { +#define LogResult(aError, ...) \ + do \ + { \ + otError _err = (aError); \ + \ + if (_err == OT_ERROR_NONE) \ + { \ + otLogInfoPlat(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), otThreadErrorToString(_err)); \ + } \ + else \ + { \ + otLogWarnPlat(OT_FIRST_ARG(__VA_ARGS__) ": %s" OT_REST_ARGS(__VA_ARGS__), otThreadErrorToString(_err)); \ + } \ + } while (false) + void MulticastRoutingManager::SetUp(void) { OT_ASSERT(gInstance != nullptr); @@ -97,7 +113,7 @@ void MulticastRoutingManager::Enable(void) InitMulticastRouterSock(); - otLogResultPlat(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); + LogResult(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); exit: return; } @@ -106,7 +122,7 @@ void MulticastRoutingManager::Disable(void) { FinalizeMulticastRouterSock(); - otLogResultPlat(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); + LogResult(OT_ERROR_NONE, "MulticastRoutingManager: %s", __FUNCTION__); } void MulticastRoutingManager::Add(const Ip6::Address &aAddress) @@ -116,7 +132,7 @@ void MulticastRoutingManager::Add(const Ip6::Address &aAddress) UnblockInboundMulticastForwardingCache(aAddress); UpdateMldReport(aAddress, true); - otLogResultPlat(OT_ERROR_NONE, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); + LogResult(OT_ERROR_NONE, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); exit: return; @@ -131,7 +147,7 @@ void MulticastRoutingManager::Remove(const Ip6::Address &aAddress) RemoveInboundMulticastForwardingCache(aAddress); UpdateMldReport(aAddress, false); - otLogResultPlat(error, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); + LogResult(error, "MulticastRoutingManager: %s: %s", __FUNCTION__, aAddress.ToString().AsCString()); exit: return; @@ -149,8 +165,8 @@ void MulticastRoutingManager::UpdateMldReport(const Ip6::Address &aAddress, bool ? OT_ERROR_FAILED : OT_ERROR_NONE); - otLogResultPlat(error, "MulticastRoutingManager: %s: address %s %s", __FUNCTION__, aAddress.ToString().AsCString(), - (isAdd ? "Added" : "Removed")); + LogResult(error, "MulticastRoutingManager: %s: address %s %s", __FUNCTION__, aAddress.ToString().AsCString(), + (isAdd ? "Added" : "Removed")); } bool MulticastRoutingManager::HasMulticastListener(const Ip6::Address &aAddress) const @@ -266,7 +282,7 @@ void MulticastRoutingManager::ProcessMulticastRouterMessages(void) error = AddMulticastForwardingCache(src, dst, static_cast(mrt6msg->im6_mif)); exit: - otLogResultPlat(error, "MulticastRoutingManager: %s", __FUNCTION__); + LogResult(error, "MulticastRoutingManager: %s", __FUNCTION__); } otError MulticastRoutingManager::AddMulticastForwardingCache(const Ip6::Address &aSrcAddr, @@ -321,9 +337,9 @@ otError MulticastRoutingManager::AddMulticastForwardingCache(const Ip6::Address SaveMulticastForwardingCache(aSrcAddr, aGroupAddr, aIif, forwardMif); exit: - otLogResultPlat(error, "MulticastRoutingManager: %s: add dynamic route: %s %s => %s %s", __FUNCTION__, - MifIndexToString(aIif), aSrcAddr.ToString().AsCString(), aGroupAddr.ToString().AsCString(), - MifIndexToString(forwardMif)); + LogResult(error, "MulticastRoutingManager: %s: add dynamic route: %s %s => %s %s", __FUNCTION__, + MifIndexToString(aIif), aSrcAddr.ToString().AsCString(), aGroupAddr.ToString().AsCString(), + MifIndexToString(forwardMif)); return error; } @@ -358,9 +374,9 @@ void MulticastRoutingManager::UnblockInboundMulticastForwardingCache(const Ip6:: mfc.Set(kMifIndexBackbone, kMifIndexThread); - otLogResultPlat(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(mfc.mIif), - mfc.mSrcAddr.ToString().AsCString(), mfc.mGroupAddr.ToString().AsCString(), - MifIndexToString(kMifIndexThread)); + LogResult(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(mfc.mIif), + mfc.mSrcAddr.ToString().AsCString(), mfc.mGroupAddr.ToString().AsCString(), + MifIndexToString(kMifIndexThread)); } } @@ -586,9 +602,9 @@ void MulticastRoutingManager::RemoveMulticastForwardingCache( ? OT_ERROR_NONE : OT_ERROR_FAILED; - otLogResultPlat(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(aMfc.mIif), - aMfc.mSrcAddr.ToString().AsCString(), aMfc.mGroupAddr.ToString().AsCString(), - MifIndexToString(aMfc.mOif)); + LogResult(error, "MulticastRoutingManager: %s: %s %s => %s %s", __FUNCTION__, MifIndexToString(aMfc.mIif), + aMfc.mSrcAddr.ToString().AsCString(), aMfc.mGroupAddr.ToString().AsCString(), + MifIndexToString(aMfc.mOif)); aMfc.Erase(); }