From d559147595c6da019f3a19a0906be02e7a59dc4f Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 9 Aug 2021 14:07:43 -0700 Subject: [PATCH] [logging] simplify logging functions (#6903) This commit simplifies the logging macros by removing the input string parameter `aRegionPrefix` in the call to the underlying log function and instead converts the `aLogRegion` to the proper string from the private `Log()` function. The removal of the additional argument in the call to the logging functions helps reduce the overall code size. This is applicable to the default and commonly used configuration where logging uses functions. In the case where logging is fully implemented as macros the `aRegionPrefix` is still needed and used. --- src/core/common/logging.cpp | 87 ++++++++++++++++--------------------- src/core/common/logging.hpp | 25 ++++++++--- 2 files changed, 55 insertions(+), 57 deletions(-) diff --git a/src/core/common/logging.cpp b/src/core/common/logging.cpp index 5c4c757b0..102a9e177 100644 --- a/src/core/common/logging.cpp +++ b/src/core/common/logging.cpp @@ -52,11 +52,7 @@ extern "C" { #if !OPENTHREAD_CONFIG_LOG_DEFINE_AS_MACRO_ONLY -static void Log(otLogLevel aLogLevel, - otLogRegion aLogRegion, - const char *aRegionPrefix, - const char *aFormat, - va_list aArgs) +static void Log(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aFormat, va_list aArgs) { ot::String logString; @@ -65,42 +61,33 @@ static void Log(otLogLevel aLogLevel, #endif #if OPENTHREAD_CONFIG_LOG_PREPEND_LEVEL + logString.Append("%s", otLogLevelToPrefixString(aLogLevel)); +#endif + +#if OPENTHREAD_CONFIG_LOG_PREPEND_REGION { - const char *levelStr = ""; + static const char *const kRegionPrefixStrings[] = { + _OT_REGION_SUFFIX, _OT_REGION_API_PREFIX, _OT_REGION_MLE_PREFIX, _OT_REGION_ARP_PREFIX, + _OT_REGION_NET_DATA_PREFIX, _OT_REGION_ICMP_PREFIX, _OT_REGION_IP6_PREFIX, _OT_REGION_MAC_PREFIX, + _OT_REGION_MEM_PREFIX, _OT_REGION_NCP_PREFIX, _OT_REGION_MESH_COP_PREFIX, _OT_REGION_NET_DIAG_PREFIX, + _OT_REGION_PLATFORM_PREFIX, _OT_REGION_COAP_PREFIX, _OT_REGION_CLI_PREFIX, _OT_REGION_CORE_PREFIX, + _OT_REGION_UTIL_PREFIX, _OT_REGION_BBR_PREFIX, _OT_REGION_MLR_PREFIX, _OT_REGION_DUA_PREFIX, + _OT_REGION_BR_PREFIX, _OT_REGION_SRP_PREFIX, _OT_REGION_DNS_PREFIX, + }; - switch (aLogLevel) + if (aLogRegion < OT_ARRAY_LENGTH(kRegionPrefixStrings)) { - case OT_LOG_LEVEL_CRIT: - levelStr = _OT_LEVEL_CRIT_PREFIX; - break; - - case OT_LOG_LEVEL_WARN: - levelStr = _OT_LEVEL_WARN_PREFIX; - break; - - case OT_LOG_LEVEL_NOTE: - levelStr = _OT_LEVEL_NOTE_PREFIX; - break; - - case OT_LOG_LEVEL_INFO: - levelStr = _OT_LEVEL_INFO_PREFIX; - break; - - case OT_LOG_LEVEL_DEBG: - levelStr = _OT_LEVEL_DEBG_PREFIX; - break; - - case OT_LOG_LEVEL_NONE: - default: - levelStr = _OT_LEVEL_NONE_PREFIX; - break; + logString.Append("%s", kRegionPrefixStrings[aLogRegion]); + } + else + { + logString.Append("%s", _OT_REGION_SUFFIX); } - - logString.Append("%s", levelStr); } -#endif // OPENTHREAD_CONFIG_LOG_PREPEND_LEVEL +#else + logString.Append("%s", _OT_REGION_SUFFIX); +#endif - logString.Append("%s", aRegionPrefix); logString.AppendVarArgs(aFormat, aArgs); otPlatLog(aLogLevel, aLogRegion, "%s" OPENTHREAD_CONFIG_LOG_SUFFIX, logString.AsCString()); @@ -111,56 +98,56 @@ exit: } #if OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_CRIT -void otLogCrit(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...) +void _otLogCrit(otLogRegion aRegion, const char *aFormat, ...) { va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_CRIT, aRegion, aRegionPrefix, aFormat, args); + Log(OT_LOG_LEVEL_CRIT, aRegion, aFormat, args); va_end(args); } #endif #if OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_WARN -void otLogWarn(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...) +void _otLogWarn(otLogRegion aRegion, const char *aFormat, ...) { va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_WARN, aRegion, aRegionPrefix, aFormat, args); + Log(OT_LOG_LEVEL_WARN, aRegion, aFormat, args); va_end(args); } #endif #if OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_NOTE -void otLogNote(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...) +void _otLogNote(otLogRegion aRegion, const char *aFormat, ...) { va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_NOTE, aRegion, aRegionPrefix, aFormat, args); + Log(OT_LOG_LEVEL_NOTE, aRegion, aFormat, args); va_end(args); } #endif #if OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_INFO -void otLogInfo(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...) +void _otLogInfo(otLogRegion aRegion, const char *aFormat, ...) { va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_INFO, aRegion, aRegionPrefix, aFormat, args); + Log(OT_LOG_LEVEL_INFO, aRegion, aFormat, args); va_end(args); } #endif #if OPENTHREAD_CONFIG_LOG_LEVEL >= OT_LOG_LEVEL_DEBG -void otLogDebg(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...) +void _otLogDebg(otLogRegion aRegion, const char *aFormat, ...) { va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_DEBG, aRegion, aRegionPrefix, aFormat, args); + Log(OT_LOG_LEVEL_DEBG, aRegion, aFormat, args); va_end(args); } #endif @@ -173,7 +160,7 @@ void otLogMac(otLogLevel aLogLevel, const char *aFormat, ...) VerifyOrExit(otLoggingGetLevel() >= aLogLevel); va_start(args, aFormat); - Log(aLogLevel, OT_LOG_REGION_MAC, _OT_REGION_MAC_PREFIX, aFormat, args); + Log(aLogLevel, OT_LOG_REGION_MAC, aFormat, args); va_end(args); exit: @@ -187,7 +174,7 @@ void otLogCertMeshCoP(const char *aFormat, ...) va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_NONE, OT_LOG_REGION_MESH_COP, _OT_REGION_MESH_COP_PREFIX, aFormat, args); + Log(OT_LOG_LEVEL_NONE, OT_LOG_REGION_MESH_COP, aFormat, args); va_end(args); } #endif @@ -198,7 +185,7 @@ void otLogOtns(const char *aFormat, ...) va_list args; va_start(args, aFormat); - Log(OT_LOG_LEVEL_NONE, OT_LOG_REGION_CORE, _OT_REGION_CORE_PREFIX, aFormat, args); + Log(OT_LOG_LEVEL_NONE, OT_LOG_REGION_CORE, aFormat, args); va_end(args); } #endif @@ -218,7 +205,7 @@ static void otLogDump(otLogLevel aLogLevel, otLogRegion aRegion, const char *aFo VerifyOrExit(otLoggingGetLevel() >= aLogLevel); va_start(args, aFormat); - Log(aLogLevel, aRegion, "", aFormat, args); + Log(aLogLevel, aRegion, aFormat, args); va_end(args); exit: @@ -316,7 +303,7 @@ void otDump(otLogLevel, otLogRegion, const char *, const void *, const size_t) } #endif // OPENTHREAD_CONFIG_LOG_PKT_DUMP -#if OPENTHREAD_CONFIG_LOG_DEFINE_AS_MACRO_ONLY +#if OPENTHREAD_CONFIG_LOG_DEFINE_AS_MACRO_ONLY || OPENTHREAD_CONFIG_LOG_PREPEND_LEVEL const char *otLogLevelToPrefixString(otLogLevel aLogLevel) { diff --git a/src/core/common/logging.hpp b/src/core/common/logging.hpp index 03e26db9f..85f536a81 100644 --- a/src/core/common/logging.hpp +++ b/src/core/common/logging.hpp @@ -97,6 +97,10 @@ extern "C" { #define _OT_REGION_BR_PREFIX "-BR------: " #define _OT_REGION_SRP_PREFIX "-SRP-----: " #define _OT_REGION_DNS_PREFIX "-DNS-----: " + +// When adding a new log region, please ensure to update the array +// `kRegionPrefixStrings[]` in `Log()` function in `logging.cpp`. + #else #define _OT_REGION_API_PREFIX _OT_REGION_SUFFIX #define _OT_REGION_MLE_PREFIX _OT_REGION_SUFFIX @@ -137,7 +141,8 @@ extern "C" { #define otLogCrit(aRegion, aRegionPrefix, ...) \ _otLogFormatter(OT_LOG_LEVEL_CRIT, aRegion, _OT_LEVEL_CRIT_PREFIX aRegionPrefix __VA_ARGS__) #else -void otLogCrit(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...); +#define otLogCrit(aRegion, aRegionPrefix, ...) _otLogCrit(aRegion, __VA_ARGS__) +void _otLogCrit(otLogRegion aRegion, const char *aFormat, ...); #endif /** @@ -155,7 +160,8 @@ void otLogCrit(otLogRegion aRegion, const char *aRegionPrefix, const char *aForm #define otLogWarn(aRegion, aRegionPrefix, ...) \ _otLogFormatter(OT_LOG_LEVEL_WARN, aRegion, _OT_LEVEL_WARN_PREFIX aRegionPrefix __VA_ARGS__) #else -void otLogWarn(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...); +#define otLogWarn(aRegion, aRegionPrefix, ...) _otLogWarn(aRegion, __VA_ARGS__) +void _otLogWarn(otLogRegion aRegion, const char *aFormat, ...); #endif /** @@ -173,7 +179,8 @@ void otLogWarn(otLogRegion aRegion, const char *aRegionPrefix, const char *aForm #define otLogNote(aRegion, aRegionPrefix, ...) \ _otLogFormatter(OT_LOG_LEVEL_NOTE, aRegion, _OT_LEVEL_NOTE_PREFIX aRegionPrefix __VA_ARGS__) #else -void otLogNote(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...); +#define otLogNote(aRegion, aRegionPrefix, ...) _otLogNote(aRegion, __VA_ARGS__) +void _otLogNote(otLogRegion aRegion, const char *aFormat, ...); #endif /** @@ -191,7 +198,8 @@ void otLogNote(otLogRegion aRegion, const char *aRegionPrefix, const char *aForm #define otLogInfo(aRegion, aRegionPrefix, ...) \ _otLogFormatter(OT_LOG_LEVEL_INFO, aRegion, _OT_LEVEL_INFO_PREFIX aRegionPrefix __VA_ARGS__) #else -void otLogInfo(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...); +#define otLogInfo(aRegion, aRegionPrefix, ...) _otLogInfo(aRegion, __VA_ARGS__) +void _otLogInfo(otLogRegion aRegion, const char *aFormat, ...); #endif /** @@ -209,7 +217,8 @@ void otLogInfo(otLogRegion aRegion, const char *aRegionPrefix, const char *aForm #define otLogDebg(aRegion, aRegionPrefix, ...) \ _otLogFormatter(OT_LOG_LEVEL_DEBG, aRegion, _OT_LEVEL_DEBG_PREFIX aRegionPrefix __VA_ARGS__) #else -void otLogDebg(otLogRegion aRegion, const char *aRegionPrefix, const char *aFormat, ...); +#define otLogDebg(aRegion, aRegionPrefix, ...) _otLogDebg(aRegion, __VA_ARGS__) +void _otLogDebg(otLogRegion aRegion, const char *aFormat, ...); #endif /** @@ -2557,8 +2566,7 @@ void otLogOtns(const char *aFormat, ...); */ void otDump(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aId, const void *aBuf, size_t aLength); -#if OPENTHREAD_CONFIG_LOG_DEFINE_AS_MACRO_ONLY - +#if OPENTHREAD_CONFIG_LOG_DEFINE_AS_MACRO_ONLY || OPENTHREAD_CONFIG_LOG_PREPEND_LEVEL /** * This function converts a log level to a prefix string for appending to log message. * @@ -2568,6 +2576,9 @@ void otDump(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aId, const * */ const char *otLogLevelToPrefixString(otLogLevel aLogLevel); +#endif + +#if OPENTHREAD_CONFIG_LOG_DEFINE_AS_MACRO_ONLY /** * Local/private macro to format the log message