From c3bb95d5f453be97c39064b9eab1db194afd4262 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 21 Oct 2022 16:26:21 -0700 Subject: [PATCH] [string] add compile-time check for `printf` style arg consistency (#8277) This commit adds `OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK` macro in `toolchain.h`. It specifies that a function or method takes `printf` style args. It asks the compiler to check the args for consistency with the passed-in format string. The commit uses the new macro in `String::Append()` and its uses are updated to ensure consistency between format string and the args. --- include/openthread/instance.h | 2 +- include/openthread/platform/toolchain.h | 27 ++++++++++++++++++++++ src/core/common/num_utils.hpp | 13 +++++++++++ src/core/common/string.hpp | 2 +- src/core/common/uptime.cpp | 14 +++++------ src/core/meshcop/meshcop.cpp | 5 ++-- src/core/radio/trel_packet.cpp | 3 ++- src/core/thread/network_data_publisher.cpp | 2 +- 8 files changed, 55 insertions(+), 13 deletions(-) diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 8accd232d..fac04a1b3 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (252) +#define OPENTHREAD_API_VERSION (253) /** * @addtogroup api-instance diff --git a/include/openthread/platform/toolchain.h b/include/openthread/platform/toolchain.h index 56109852d..d8b0308ef 100644 --- a/include/openthread/platform/toolchain.h +++ b/include/openthread/platform/toolchain.h @@ -110,6 +110,24 @@ extern "C" { * */ +/** + * @def OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK + * + * This macro specifies that a function or method takes `printf` style arguments and should be type-checked against + * a format string. + * + * This macro must be added after the function/method declaration. For example: + * + * `void MyPrintf(void *aObject, const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 3);` + * + * The two argument index values indicate format string and first argument to check against it. They start at index 1 + * for the first parameter in a function and at index 2 for the first parameter in a method. + * + * @param[in] aFmtIndex The argument index of the format string. + * @param[in] aStartIndex The argument index of the first argument to check against the format string. + * + */ + // =========== TOOLCHAIN SELECTION : START =========== #if defined(__GNUC__) || defined(__clang__) || defined(__CC_ARM) || defined(__TI_ARM__) @@ -122,6 +140,9 @@ extern "C" { #define OT_TOOL_PACKED_END __attribute__((packed)) #define OT_TOOL_WEAK __attribute__((weak)) +#define OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(aFmtIndex, aStartIndex) \ + __attribute__((format(printf, aFmtIndex, aStartIndex))) + #elif defined(__ICCARM__) || defined(__ICC8051__) // http://supp.iar.com/FilesPublic/UPDINFO/004916/arm/doc/EWARM_DevelopmentGuide.ENU.pdf @@ -133,6 +154,8 @@ extern "C" { #define OT_TOOL_PACKED_END #define OT_TOOL_WEAK __weak +#define OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(aFmtIndex, aStartIndex) + #elif defined(__SDCC) // Structures are packed by default in sdcc, as it primarily targets 8-bit MCUs. @@ -142,6 +165,8 @@ extern "C" { #define OT_TOOL_PACKED_END #define OT_TOOL_WEAK +#define OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(aFmtIndex, aStartIndex) + #else #error "Error: No valid Toolchain specified" @@ -153,6 +178,8 @@ extern "C" { #define OT_TOOL_PACKED_END #define OT_TOOL_WEAK +#define OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(aFmtIndex, aStartIndex) + #endif // =========== TOOLCHAIN SELECTION : END =========== diff --git a/src/core/common/num_utils.hpp b/src/core/common/num_utils.hpp index 3d479b9a9..d53134d6f 100644 --- a/src/core/common/num_utils.hpp +++ b/src/core/common/num_utils.hpp @@ -187,6 +187,19 @@ template inline IntType DivideAndRoundToClosest(IntType aDivi return (aDividend + (aDivisor / 2)) / aDivisor; } +/** + * This function casts a given `uint32_t` to `unsigned long`. + * + * @param[in] aUint32 A `uint32_t` value. + * + * @returns The @p aUint32 value as `unsigned long`. + * + */ +inline unsigned long ToUlong(uint32_t aUint32) +{ + return static_cast(aUint32); +} + } // namespace ot #endif // NUM_UTILS_HPP_ diff --git a/src/core/common/string.hpp b/src/core/common/string.hpp index ac608b3f0..c83a127aa 100644 --- a/src/core/common/string.hpp +++ b/src/core/common/string.hpp @@ -313,7 +313,7 @@ public: * @returns The string writer. * */ - StringWriter &Append(const char *aFormat, ...); + StringWriter &Append(const char *aFormat, ...) OT_TOOL_PRINTF_STYLE_FORMAT_ARG_CHECK(2, 3); /** * This method appends `printf()` style formatted data to the buffer. diff --git a/src/core/common/uptime.cpp b/src/core/common/uptime.cpp index bd3239af0..669b1ffe4 100644 --- a/src/core/common/uptime.cpp +++ b/src/core/common/uptime.cpp @@ -98,7 +98,7 @@ void Uptime::HandleTimer(void) mTimer.FireAt(mTimer.GetFireTime() + kTimerInterval); } -static uint32_t DivideAndGetRemainder(uint32_t &aDividend, uint32_t aDivisor) +static uint16_t DivideAndGetRemainder(uint32_t &aDividend, uint32_t aDivisor) { // Returns the quotient of division `aDividend / aDivisor` and updates // `aDividend` to returns the remainder @@ -107,20 +107,20 @@ static uint32_t DivideAndGetRemainder(uint32_t &aDividend, uint32_t aDivisor) aDividend -= quotient * aDivisor; - return quotient; + return static_cast(quotient); } void Uptime::UptimeToString(uint64_t aUptime, StringWriter &aWriter) { uint64_t days = aUptime / Time::kOneDayInMsec; uint32_t remainder; - uint32_t hours; - uint32_t minutes; - uint32_t seconds; + uint16_t hours; + uint16_t minutes; + uint16_t seconds; if (days > 0) { - aWriter.Append("%lud.", days); + aWriter.Append("%lud.", static_cast(days)); aUptime -= days * Time::kOneDayInMsec; } @@ -129,7 +129,7 @@ void Uptime::UptimeToString(uint64_t aUptime, StringWriter &aWriter) minutes = DivideAndGetRemainder(remainder, Time::kOneMinuteInMsec); seconds = DivideAndGetRemainder(remainder, Time::kOneSecondInMsec); - aWriter.Append("%02u:%02u:%02u.%03u", hours, minutes, seconds, remainder); + aWriter.Append("%02u:%02u:%02u.%03u", hours, minutes, seconds, static_cast(remainder)); } } // namespace ot diff --git a/src/core/meshcop/meshcop.cpp b/src/core/meshcop/meshcop.cpp index a61b0ae31..80bb7b44f 100644 --- a/src/core/meshcop/meshcop.cpp +++ b/src/core/meshcop/meshcop.cpp @@ -174,11 +174,12 @@ JoinerDiscerner::InfoString JoinerDiscerner::ToString(void) const } else if (mLength <= sizeof(uint32_t) * CHAR_BIT) { - string.Append("0x%08x", static_cast(mValue)); + string.Append("0x%08lx", ToUlong(static_cast(mValue))); } else { - string.Append("0x%x-%08x", static_cast(mValue >> 32), static_cast(mValue)); + string.Append("0x%lx-%08lx", ToUlong(static_cast(mValue >> 32)), + ToUlong(static_cast(mValue))); } string.Append("/len:%d", mLength); diff --git a/src/core/radio/trel_packet.cpp b/src/core/radio/trel_packet.cpp index 9b5cabdfd..2847b6001 100644 --- a/src/core/radio/trel_packet.cpp +++ b/src/core/radio/trel_packet.cpp @@ -94,7 +94,8 @@ Header::InfoString Header::ToString(void) const break; } - string.Append(" panid:%04x num:%lu src:%s", GetPanId(), GetPacketNumber(), GetSource().ToString().AsCString()); + string.Append(" panid:%04x num:%lu src:%s", GetPanId(), ToUlong(GetPacketNumber()), + GetSource().ToString().AsCString()); if ((type == kTypeUnicast) || (type == kTypeAck)) { diff --git a/src/core/thread/network_data_publisher.cpp b/src/core/thread/network_data_publisher.cpp index 4c79ef305..71b6daa30 100644 --- a/src/core/thread/network_data_publisher.cpp +++ b/src/core/thread/network_data_publisher.cpp @@ -456,7 +456,7 @@ Publisher::Entry::InfoString Publisher::Entry::ToString(bool aIncludeState) cons break; } - string.Append(prefixEntry.mPrefix.ToString().AsCString()); + string.Append("%s", prefixEntry.mPrefix.ToString().AsCString()); ExitNow(); } #endif