From 40e693762d762dac3e679c107d74e48a827330af Mon Sep 17 00:00:00 2001 From: Yakun Xu Date: Wed, 24 Dec 2025 11:31:08 +0800 Subject: [PATCH] [build] add `printf` literal string format checks for `va_list` functions (#12236) This commit introduces enhanced format string checking. It activates a new compiler warning to identify potential issues with non-literal format strings and systematically applies format attribute macros to functions that handle variable arguments. --- etc/gn/toolchain/BUILD.gn | 1 + include/openthread/cli.h | 6 ++++-- include/openthread/instance.h | 2 +- include/openthread/logging.h | 3 ++- src/cli/cli_utils.hpp | 4 ++-- src/core/common/log.hpp | 3 ++- src/core/common/string.hpp | 2 +- src/lib/spinel/logger.cpp | 13 ++++++++----- src/lib/spinel/logger.hpp | 5 +++-- third_party/tcplp/tcplp.h | 4 ++-- 10 files changed, 26 insertions(+), 17 deletions(-) diff --git a/etc/gn/toolchain/BUILD.gn b/etc/gn/toolchain/BUILD.gn index a5a54747b..a67f1fd05 100644 --- a/etc/gn/toolchain/BUILD.gn +++ b/etc/gn/toolchain/BUILD.gn @@ -31,6 +31,7 @@ toolchain("toolchain") { "-Wall", "-Wextra", "-Wundef", + "-Wformat-nonliteral", ] if (use_clang) { cc = "clang" diff --git a/include/openthread/cli.h b/include/openthread/cli.h index ef87de188..bd0941244 100644 --- a/include/openthread/cli.h +++ b/include/openthread/cli.h @@ -41,6 +41,7 @@ #include #include #include +#include #ifdef __cplusplus extern "C" { @@ -119,7 +120,7 @@ void otCliOutputBytes(const uint8_t *aBytes, uint8_t aLength); * @param[in] aFmt A pointer to the format string. * @param[in] ... A matching list of arguments. */ -void otCliOutputFormat(const char *aFmt, ...); +void otCliOutputFormat(const char *aFmt, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(1, 2); /** * Write error code to the CLI console @@ -138,7 +139,8 @@ void otCliAppendResult(otError aError); * @param[in] aFormat A pointer to the format string. * @param[in] aArgs va_list matching aFormat. */ -void otCliPlatLogv(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aFormat, va_list aArgs); +void otCliPlatLogv(otLogLevel aLogLevel, otLogRegion aLogRegion, const char *aFormat, va_list aArgs) + OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(3, 0); /** * Callback to allow vendor specific commands to be added to the user command table. diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 4f3c82371..1b3c27e95 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -52,7 +52,7 @@ extern "C" { * * @note This number versions both OpenThread platform and user APIs. */ -#define OPENTHREAD_API_VERSION (564) +#define OPENTHREAD_API_VERSION (565) /** * @addtogroup api-instance diff --git a/include/openthread/logging.h b/include/openthread/logging.h index da6dbd677..74f08d913 100644 --- a/include/openthread/logging.h +++ b/include/openthread/logging.h @@ -227,7 +227,8 @@ void otLogPlat(otLogLevel aLogLevel, const char *aPlatModuleName, const char *aF * @param[in] aFormat The format string. * @param[in] aArgs Arguments for the format specification. */ -void otLogPlatArgs(otLogLevel aLogLevel, const char *aPlatModuleName, const char *aFormat, va_list aArgs); +void otLogPlatArgs(otLogLevel aLogLevel, const char *aPlatModuleName, const char *aFormat, va_list aArgs) + OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(3, 0); /** * Emits a log message at a given log level. diff --git a/src/cli/cli_utils.hpp b/src/cli/cli_utils.hpp index f92579f6a..8ebbc86b0 100644 --- a/src/cli/cli_utils.hpp +++ b/src/cli/cli_utils.hpp @@ -99,7 +99,7 @@ public: private: static constexpr uint16_t kInputOutputLogStringSize = OPENTHREAD_CONFIG_CLI_LOG_INPUT_OUTPUT_LOG_STRING_SIZE; - void OutputV(const char *aFormat, va_list aArguments); + void OutputV(const char *aFormat, va_list aArguments) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 0); otCliOutputCallback mCallback; void *mCallbackContext; @@ -728,7 +728,7 @@ public: static const char *BorderRoutingStateToString(otBorderRoutingState aState); protected: - void OutputFormatV(const char *aFormat, va_list aArguments); + void OutputFormatV(const char *aFormat, va_list aArguments) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 0); #if OPENTHREAD_CONFIG_CLI_LOG_INPUT_OUTPUT_ENABLE void LogInput(const Arg *aArgs); diff --git a/src/core/common/log.hpp b/src/core/common/log.hpp index b43f24132..e4511ceae 100644 --- a/src/core/common/log.hpp +++ b/src/core/common/log.hpp @@ -312,7 +312,8 @@ public: static void LogAtLevel(const char *aModuleName, const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 3); - static void LogVarArgs(const char *aModuleName, LogLevel aLogLevel, const char *aFormat, va_list aArgs); + static void LogVarArgs(const char *aModuleName, LogLevel aLogLevel, const char *aFormat, va_list aArgs) + OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(3, 0); #if OT_SHOULD_LOG_AT(OT_LOG_LEVEL_WARN) static void LogOnError(const char *aModuleName, Error aError, const char *aText); diff --git a/src/core/common/string.hpp b/src/core/common/string.hpp index d8c3adf99..6c9202bc5 100644 --- a/src/core/common/string.hpp +++ b/src/core/common/string.hpp @@ -461,7 +461,7 @@ public: * * @returns The string writer. */ - StringWriter &AppendVarArgs(const char *aFormat, va_list aArgs); + StringWriter &AppendVarArgs(const char *aFormat, va_list aArgs) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 0); /** * Appends an array of bytes in hex representation (using "%02x" style) to the buffer. diff --git a/src/lib/spinel/logger.cpp b/src/lib/spinel/logger.cpp index 081c71e3c..d1cb86178 100644 --- a/src/lib/spinel/logger.cpp +++ b/src/lib/spinel/logger.cpp @@ -311,7 +311,7 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx) VerifyOrExit(unpacked > 0, error = OT_ERROR_PARSE); name = (key == SPINEL_PROP_RCP_TIMESTAMP) ? "timestamp" : "counter"; - start += Snprintf(start, static_cast(end - start), ", %s:%u", name, value); + start += Snprintf(start, static_cast(end - start), ", %s:%u", name, static_cast(value)); } break; @@ -422,7 +422,8 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx) maskLength -= static_cast(unpacked); } - start += Snprintf(start, static_cast(end - start), ", channelMask:0x%08x", channelMask); + start += Snprintf(start, static_cast(end - start), ", channelMask:0x%08x", + static_cast(channelMask)); } break; @@ -507,8 +508,9 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx) start += Snprintf(start, static_cast(end - start), "... csmaCaEnabled:%u, isHeaderUpdated:%u, isARetx:%u, skipAes:%u" ", txDelay:%u, txDelayBase:%u", - csmaCaEnabled, isHeaderUpdated, isARetx, skipAes, frame.mInfo.mTxInfo.mTxDelay, - frame.mInfo.mTxInfo.mTxDelayBaseTime); + csmaCaEnabled, isHeaderUpdated, isARetx, skipAes, + static_cast(frame.mInfo.mTxInfo.mTxDelay), + static_cast(frame.mInfo.mTxInfo.mTxDelayBaseTime)); } } break; @@ -672,7 +674,8 @@ void Logger::LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx) LogDebg(" stopped:%u", metrics.mStopped); start = buf; - start += Snprintf(start, static_cast(end - start), " grantGlitch:%u", metrics.mNumGrantGlitch); + start += Snprintf(start, static_cast(end - start), " grantGlitch:%u", + static_cast(metrics.mNumGrantGlitch)); } break; diff --git a/src/lib/spinel/logger.hpp b/src/lib/spinel/logger.hpp index 9ab3ff9fb..d98fb0b6d 100644 --- a/src/lib/spinel/logger.hpp +++ b/src/lib/spinel/logger.hpp @@ -50,8 +50,9 @@ protected: void LogInfo(const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 3); void LogDebg(const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 3); - uint32_t Snprintf(char *aDest, uint32_t aSize, const char *aFormat, ...); - void LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx); + uint32_t Snprintf(char *aDest, uint32_t aSize, const char *aFormat, ...) + OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(4, 5); + void LogSpinelFrame(const uint8_t *aFrame, uint16_t aLength, bool aTx); enum { diff --git a/third_party/tcplp/tcplp.h b/third_party/tcplp/tcplp.h index d45ee3589..71a3f8af8 100644 --- a/third_party/tcplp/tcplp.h +++ b/third_party/tcplp/tcplp.h @@ -81,8 +81,8 @@ bool tcplp_sys_accepted_connection(struct tcpcb_listen *aTcbListen, uint16_t aPort); void tcplp_sys_connection_lost(struct tcpcb *aTcb, uint8_t aErrNum); void tcplp_sys_on_state_change(struct tcpcb *aTcb, int aNewState); -void tcplp_sys_log(const char *aFormat, ...); -void tcplp_sys_panic(const char *aFormat, ...); +void tcplp_sys_log(const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(1, 2); +void tcplp_sys_panic(const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(1, 2); bool tcplp_sys_autobind(otInstance * aInstance, const otSockAddr *aPeer, otSockAddr * aToBind,