From 26f23ad635f7a9ec8277bb079ebabb42421a52b3 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Mon, 14 Jul 2025 13:06:27 -0700 Subject: [PATCH] [core] fix implicit integer conversion warnings (#11709) This commit enables the `-Wimplicit-int-conversion` compiler flag for `ftd`, `mtd`, and `radio` to improve code quality by detecting potential data loss from implicit type conversions. This is enabled when clang toolchain is used. All resulting warnings have been addressed by either: - Changing variable, parameter, or return types to ensure consistency and prevent overflows. - Adding explicit `static_cast` where the type conversion is intended and safe. --- src/core/border_router/dhcp6_pd_client.cpp | 2 +- src/core/border_router/routing_manager.cpp | 4 ++-- src/core/border_router/routing_manager.hpp | 2 +- src/core/common/num_utils.hpp | 2 +- src/core/diags/factory_diags.cpp | 12 ++++++------ src/core/diags/factory_diags.hpp | 2 +- src/core/ftd.cmake | 4 ++++ src/core/meshcop/meshcop_tlvs.hpp | 5 ++++- src/core/mtd.cmake | 4 ++++ src/core/net/dhcp6_types.hpp | 6 +++--- src/core/net/dns_client.cpp | 2 +- src/core/net/dns_types.cpp | 4 ++-- src/core/net/dns_types.hpp | 2 +- src/core/net/ip4_types.hpp | 2 +- src/core/net/ip6.hpp | 2 +- src/core/net/mdns.cpp | 2 +- src/core/net/mdns.hpp | 10 +++++----- src/core/net/nd6.hpp | 4 ++-- src/core/radio.cmake | 4 ++++ src/core/radio/trel_interface.cpp | 2 +- src/core/thread/network_diagnostic_tlvs.hpp | 5 ++++- tests/unit/test_routing_manager.cpp | 2 +- 22 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/core/border_router/dhcp6_pd_client.cpp b/src/core/border_router/dhcp6_pd_client.cpp index 1f756e83c..5059da69c 100644 --- a/src/core/border_router/dhcp6_pd_client.cpp +++ b/src/core/border_router/dhcp6_pd_client.cpp @@ -696,7 +696,7 @@ void Dhcp6PdClient::SaveServerDuidAndAddress(const Message &aMessage) Ip6::Address serverAddress; SuccessOrAssert(ServerIdOption::ReadDuid(aMessage, serverDuidOffsetRange)); - mServerDuid.SetLength(serverDuidOffsetRange.GetLength()); + mServerDuid.SetLength(static_cast(serverDuidOffsetRange.GetLength())); aMessage.ReadBytes(serverDuidOffsetRange, mServerDuid.GetArrayBuffer()); ProcessServerUnicastOption(aMessage, serverAddress); diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp index e5c7d8dc7..0b2757ed5 100644 --- a/src/core/border_router/routing_manager.cpp +++ b/src/core/border_router/routing_manager.cpp @@ -1078,7 +1078,7 @@ void RoutingManager::RoutePrefix::CopyInfoTo(PrefixTableEntry &aEntry, TimeMilli //--------------------------------------------------------------------------------------------------------------------- // RdnssAddress -void RoutingManager::RdnssAddress::SetFrom(const RecursiveDnsServerOption &aRdnss, uint8_t aAddressIndex) +void RoutingManager::RdnssAddress::SetFrom(const RecursiveDnsServerOption &aRdnss, uint16_t aAddressIndex) { mAddress = aRdnss.GetAddressAt(aAddressIndex); mLifetime = aRdnss.GetLifetime(); @@ -1564,7 +1564,7 @@ void RoutingManager::RxRaTracker::ProcessRecursiveDnsServerOption(const Recursiv lifetime = aRdnss.GetLifetime(); - for (uint8_t index = 0; index < aRdnss.GetNumAddresses(); index++) + for (uint16_t index = 0; index < aRdnss.GetNumAddresses(); index++) { const Ip6::Address &address = aRdnss.GetAddressAt(index); diff --git a/src/core/border_router/routing_manager.hpp b/src/core/border_router/routing_manager.hpp index d1c6038c0..c011d8af2 100644 --- a/src/core/border_router/routing_manager.hpp +++ b/src/core/border_router/routing_manager.hpp @@ -893,7 +893,7 @@ private: class RdnssAddress { public: - void SetFrom(const RecursiveDnsServerOption &aRdnss, uint8_t aAddressIndex); + void SetFrom(const RecursiveDnsServerOption &aRdnss, uint16_t aAddressIndex); const Ip6::Address &GetAddress(void) const { return mAddress; } const TimeMilli &GetLastUpdateTime(void) const { return mLastUpdateTime; } uint32_t GetLifetime(void) const { return mLifetime; } diff --git a/src/core/common/num_utils.hpp b/src/core/common/num_utils.hpp index 9f1df9d1f..6d031f4c6 100644 --- a/src/core/common/num_utils.hpp +++ b/src/core/common/num_utils.hpp @@ -272,7 +272,7 @@ template void SetBit(UintType &aBits, uint8_t aBitOffset) { static_assert(TypeTraits::IsUint::kValue, "UintType must be an unsigned int (8, 16, 32, or 64 bit len)"); - aBits = aBits | (static_cast(1) << aBitOffset); + aBits = aBits | static_cast(static_cast(1) << aBitOffset); } /** diff --git a/src/core/diags/factory_diags.cpp b/src/core/diags/factory_diags.cpp index e8595ad3b..f6d92200a 100644 --- a/src/core/diags/factory_diags.cpp +++ b/src/core/diags/factory_diags.cpp @@ -394,7 +394,7 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[]) else { uint32_t txPeriod; - uint8_t txLength; + uint16_t txLength; VerifyOrExit(aArgsLength >= 1, error = kErrorInvalidArgs); VerifyOrExit(mCurTxCmd == kTxCmdNone, error = kErrorInvalidState); @@ -404,7 +404,7 @@ Error Diags::ProcessRepeat(uint8_t aArgsLength, char *aArgs[]) if (aArgsLength >= 2) { - SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint8(aArgs[1], txLength)); + SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint16(aArgs[1], txLength)); mIsTxPacketSet = false; } else if (mIsTxPacketSet) @@ -431,7 +431,7 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[]) { Error error = kErrorNone; uint32_t txPackets; - uint8_t txLength; + uint16_t txLength; VerifyOrExit(aArgsLength >= 1, error = kErrorInvalidArgs); VerifyOrExit(mCurTxCmd == kTxCmdNone, error = kErrorInvalidState); @@ -453,7 +453,7 @@ Error Diags::ProcessSend(uint8_t aArgsLength, char *aArgs[]) if (aArgsLength >= 2) { - SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint8(aArgs[1], txLength)); + SuccessOrExit(error = Utils::CmdLineParser::ParseAsUint16(aArgs[1], txLength)); mIsTxPacketSet = false; } else if (mIsTxPacketSet) @@ -612,9 +612,9 @@ Error Diags::TransmitPacket(void) ResetTxPacket(); mTxPacket->mLength = mTxLen; - for (uint8_t i = 0; i < mTxLen; i++) + for (uint16_t i = 0; i < mTxLen; i++) { - mTxPacket->mPsdu[i] = i; + mTxPacket->mPsdu[i] = static_cast(i & 0xff); } } diff --git a/src/core/diags/factory_diags.hpp b/src/core/diags/factory_diags.hpp index 71b262f6c..53faaf63d 100644 --- a/src/core/diags/factory_diags.hpp +++ b/src/core/diags/factory_diags.hpp @@ -264,7 +264,7 @@ private: uint32_t mTxPackets; uint8_t mChannel; int8_t mTxPower; - uint8_t mTxLen; + uint16_t mTxLen; TxCmd mCurTxCmd; bool mIsHeaderUpdated : 1; bool mIsSecurityProcessed : 1; diff --git a/src/core/ftd.cmake b/src/core/ftd.cmake index 2e16087cd..e3e3462a5 100644 --- a/src/core/ftd.cmake +++ b/src/core/ftd.cmake @@ -39,6 +39,10 @@ target_compile_options(openthread-ftd PRIVATE -Wundef ) +if("${CMAKE_CXX_COMPILER_ID}" MATCHES "AppleClang") + target_compile_options(openthread-ftd PRIVATE -Wimplicit-int-conversion) +endif() + target_include_directories(openthread-ftd PUBLIC ${OT_PUBLIC_INCLUDES} PRIVATE ${COMMON_INCLUDES}) target_sources(openthread-ftd PRIVATE ${COMMON_SOURCES}) diff --git a/src/core/meshcop/meshcop_tlvs.hpp b/src/core/meshcop/meshcop_tlvs.hpp index 1c472fbec..d4331ce41 100644 --- a/src/core/meshcop/meshcop_tlvs.hpp +++ b/src/core/meshcop/meshcop_tlvs.hpp @@ -851,7 +851,10 @@ public: * * @returns The Revision value. */ - uint8_t GetRevision(void) const { return ReadBitsBigEndian(mBuildRevision); } + uint8_t GetRevision(void) const + { + return static_cast(ReadBitsBigEndian(mBuildRevision)); + } /** * Sets the Revision value. diff --git a/src/core/mtd.cmake b/src/core/mtd.cmake index ebef0d3b4..1a4f71b45 100644 --- a/src/core/mtd.cmake +++ b/src/core/mtd.cmake @@ -39,6 +39,10 @@ target_compile_options(openthread-mtd PRIVATE -Wundef ) +if("${CMAKE_CXX_COMPILER_ID}" MATCHES "AppleClang") + target_compile_options(openthread-mtd PRIVATE -Wimplicit-int-conversion) +endif() + target_include_directories(openthread-mtd PUBLIC ${OT_PUBLIC_INCLUDES} PRIVATE ${COMMON_INCLUDES}) target_sources(openthread-mtd PRIVATE ${COMMON_SOURCES}) diff --git a/src/core/net/dhcp6_types.hpp b/src/core/net/dhcp6_types.hpp index 245a1ba00..ce2c862db 100644 --- a/src/core/net/dhcp6_types.hpp +++ b/src/core/net/dhcp6_types.hpp @@ -725,14 +725,14 @@ public: * * @returns The preference value. Higher value is preferred. */ - uint16_t GetPreference(void) const { return mPreference; } + uint8_t GetPreference(void) const { return mPreference; } /** * Sets the preference. * * @param[in] aPreference The preference value. */ - void SetPreference(uint16_t aPreference) { mPreference = aPreference; } + void SetPreference(uint8_t aPreference) { mPreference = aPreference; } private: uint8_t mPreference; @@ -1069,7 +1069,7 @@ public: * * @returns The SOL_MAX_RT value (in seconds). */ - uint16_t GetSolMaxRt(void) const { return BigEndian::HostSwap32(mSolMaxRt); } + uint32_t GetSolMaxRt(void) const { return BigEndian::HostSwap32(mSolMaxRt); } /** * Sets the SOL_MAX_RT. diff --git a/src/core/net/dns_client.cpp b/src/core/net/dns_client.cpp index d5a242e81..c138a3c57 100644 --- a/src/core/net/dns_client.cpp +++ b/src/core/net/dns_client.cpp @@ -1271,7 +1271,7 @@ uint16_t Client::DetermineQuestionRecordType(const QueryInfo &aInfo) const // Determine the first record type to include in Question // section based on the `mQueryType`. - uint8_t recordType = 0; + uint16_t recordType = 0; switch (aInfo.mQueryType) { diff --git a/src/core/net/dns_types.cpp b/src/core/net/dns_types.cpp index d3bfd6c10..8db30eff1 100644 --- a/src/core/net/dns_types.cpp +++ b/src/core/net/dns_types.cpp @@ -1390,7 +1390,7 @@ exit: return error; } -Error TxtDataEncoder::AppendBytesEntry(const char *aKey, const void *aBuffer, uint16_t aLength) +Error TxtDataEncoder::AppendBytesEntry(const char *aKey, const void *aBuffer, uint8_t aLength) { return TxtEntry(aKey, reinterpret_cast(aBuffer), aLength).AppendTo(mAppender); } @@ -1402,7 +1402,7 @@ Error TxtDataEncoder::AppendStringEntry(const char *aKey, const char *aStringVal VerifyOrExit(length <= kMaxStringEntryLength, error = kErrorInvalidArgs); - error = AppendBytesEntry(aKey, aStringValue, length); + error = AppendBytesEntry(aKey, aStringValue, static_cast(length)); exit: return error; diff --git a/src/core/net/dns_types.hpp b/src/core/net/dns_types.hpp index 7e414440a..9c8b93d5a 100644 --- a/src/core/net/dns_types.hpp +++ b/src/core/net/dns_types.hpp @@ -1304,7 +1304,7 @@ public: * @retval kErrorNone Successfully appended the TXT entry. * @retval kErrorNoBufs Insufficient available buffers to append the entry. */ - Error AppendBytesEntry(const char *aKey, const void *aBuffer, uint16_t aLength); + Error AppendBytesEntry(const char *aKey, const void *aBuffer, uint8_t aLength); /** * Appends a TXT entry for a given key and a given object as the entry's value. diff --git a/src/core/net/ip4_types.hpp b/src/core/net/ip4_types.hpp index 2743687ad..fd1288adf 100644 --- a/src/core/net/ip4_types.hpp +++ b/src/core/net/ip4_types.hpp @@ -705,7 +705,7 @@ public: * * @returns The IPv4 header Total length value. */ - uint8_t GetIpLength(void) const { return mIp4Header.GetTotalLength(); } + uint16_t GetIpLength(void) const { return mIp4Header.GetTotalLength(); } /** * Returns the IPv4 TTL value. diff --git a/src/core/net/ip6.hpp b/src/core/net/ip6.hpp index 9980fd8b4..80130b36a 100644 --- a/src/core/net/ip6.hpp +++ b/src/core/net/ip6.hpp @@ -487,7 +487,7 @@ public: * * @returns The IPv6 Payload Length value. */ - uint8_t GetIpLength(void) const { return mIp6Header.GetPayloadLength(); } + uint16_t GetIpLength(void) const { return mIp6Header.GetPayloadLength(); } /** * Returns the IPv6 Hop Limit value. diff --git a/src/core/net/mdns.cpp b/src/core/net/mdns.cpp index 7d9eacbfb..07347aaba 100644 --- a/src/core/net/mdns.cpp +++ b/src/core/net/mdns.cpp @@ -4140,7 +4140,7 @@ Core::RxMessage::ProcessOutcome Core::RxMessage::ProcessQuery(bool aShouldProces if (shouldDelay) { - delay = Random::NonCrypto::GetUint32InRange(kMinResponseDelay, kMaxResponseDelay); + delay = Random::NonCrypto::GetUint16InRange(kMinResponseDelay, kMaxResponseDelay); } for (const Question &question : mQuestions) diff --git a/src/core/net/mdns.hpp b/src/core/net/mdns.hpp index c78af1224..5fe345bc3 100644 --- a/src/core/net/mdns.hpp +++ b/src/core/net/mdns.hpp @@ -865,8 +865,8 @@ private: static constexpr uint32_t kMaxInitialQueryDelay = 120; // msec static constexpr uint32_t kRandomDelayReuseInterval = 2; // msec - static constexpr uint32_t kMinResponseDelay = 20; // msec - static constexpr uint32_t kMaxResponseDelay = 120; // msec + static constexpr uint16_t kMinResponseDelay = 20; // msec + static constexpr uint16_t kMaxResponseDelay = 120; // msec static constexpr uint32_t kResponseAggregationMaxDelay = 500; // msec static constexpr uint32_t kUnspecifiedTtl = 0; @@ -980,7 +980,7 @@ private: TimeMilli GetAnswerTime(void) const { return (mQueryRxTime + mAnswerDelay); } uint16_t mQuestionRrType; - uint16_t mAnswerDelay; + uint32_t mAnswerDelay; TimeMilli mQueryRxTime; bool mIsProbe; bool mUnicastResponse; @@ -1085,7 +1085,7 @@ private: uint8_t mAnnounceCounter; AppendState mAppendState; Section mAppendSection; - uint16_t mAnswerDelay; + uint32_t mAnswerDelay; uint32_t mTtl; TimeMilli mAnnounceTime; TimeMilli mQueryRxTime; @@ -1179,7 +1179,7 @@ private: bool mUnicastNsecPending : 1; bool mAppendedNsec : 1; bool mBypassCallbackStateCheck : 1; - uint16_t mNsecAnswerDelay; + uint32_t mNsecAnswerDelay; TimeMilli mNsecQueryRxTime; Heap::Data mKeyData; Callback mCallback; diff --git a/src/core/net/nd6.hpp b/src/core/net/nd6.hpp index 8ff0899a3..4aef74166 100644 --- a/src/core/net/nd6.hpp +++ b/src/core/net/nd6.hpp @@ -534,7 +534,7 @@ public: * * @returns Number of IPv6 addresses. */ - uint8_t GetNumAddresses(void) const { return IsValid() ? (GetLength() - 1) / 2 : 0; } + uint16_t GetNumAddresses(void) const { return IsValid() ? (GetLength() - 1) / 2 : 0; } /** * Returns a pointer to array of IPv6 addresses of DNS server. @@ -563,7 +563,7 @@ public: * * @returns The IPv6 address at @p aIndex. */ - const Address &GetAddressAt(uint8_t aIndex) const { return GetAddresses()[aIndex]; } + const Address &GetAddressAt(uint16_t aIndex) const { return GetAddresses()[aIndex]; } /** * Calculates the option length for a given number of IPv6 addresses. diff --git a/src/core/radio.cmake b/src/core/radio.cmake index 001e54b8b..b1974f88b 100644 --- a/src/core/radio.cmake +++ b/src/core/radio.cmake @@ -40,6 +40,10 @@ target_compile_options(openthread-radio PRIVATE -Wundef ) +if("${CMAKE_CXX_COMPILER_ID}" MATCHES "AppleClang") + target_compile_options(openthread-radio PRIVATE -Wimplicit-int-conversion) +endif() + target_include_directories(openthread-radio PUBLIC ${OT_PUBLIC_INCLUDES} PRIVATE ${COMMON_INCLUDES}) target_sources(openthread-radio PRIVATE diff --git a/src/core/radio/trel_interface.cpp b/src/core/radio/trel_interface.cpp index ac64ffbc2..4c7d3f42f 100644 --- a/src/core/radio/trel_interface.cpp +++ b/src/core/radio/trel_interface.cpp @@ -123,7 +123,7 @@ Error Interface::Send(Packet &aPacket, bool aIsDiscovery) case Header::kTypeBroadcast: for (const Peer &peer : Get()) { - uint16_t originalPacketNumber = aPacket.GetHeader().GetPacketNumber(); + uint32_t originalPacketNumber = aPacket.GetHeader().GetPacketNumber(); Header::AckMode originalAckMode = aPacket.GetHeader().GetAckMode(); Neighbor *neighbor; diff --git a/src/core/thread/network_diagnostic_tlvs.hpp b/src/core/thread/network_diagnostic_tlvs.hpp index 00376f2ee..c7689afd2 100644 --- a/src/core/thread/network_diagnostic_tlvs.hpp +++ b/src/core/thread/network_diagnostic_tlvs.hpp @@ -495,7 +495,10 @@ public: * * @returns The Timeout value. */ - uint8_t GetTimeout(void) const { return ReadBits(GetTimeoutChildId()); } + uint8_t GetTimeout(void) const + { + return static_cast(ReadBits(GetTimeoutChildId())); + } /** * Sets the Timeout value. diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp index e9f70ab66..4d969e563 100644 --- a/tests/unit/test_routing_manager.cpp +++ b/tests/unit/test_routing_manager.cpp @@ -619,7 +619,7 @@ void LogRouterAdvert(const Icmp6Packet &aPacket) VerifyOrQuit(rdnss.IsValid()); - for (uint8_t index = 0; index < rdnss.GetNumAddresses(); index++) + for (uint16_t index = 0; index < rdnss.GetNumAddresses(); index++) { Log(" RDNSS - %s, lifetime:%u", rdnss.GetAddressAt(index).ToString().AsCString(), rdnss.GetLifetime());