From 454600016857659f8e062d4549fd33ea7c368c87 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Wed, 1 Mar 2023 11:14:34 -0500 Subject: [PATCH] Enable -Wconversion for ESP32 platform bits. (#25373) * Enable -Wconversion for ESP32 platform bits. * Address review comment. --- src/platform/ESP32/BUILD.gn | 2 ++ .../ESP32/DiagnosticDataProviderImpl.cpp | 13 +++++++++- .../ESP32/NetworkCommissioningDriver.cpp | 26 +++++++++++++++---- .../ESP32/NetworkCommissioningDriver.h | 9 ++++--- .../ESP32/route_hook/ESP32RouteHook.c | 21 +++++++++------ 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/platform/ESP32/BUILD.gn b/src/platform/ESP32/BUILD.gn index 59ab8d1dcafb1c..29d3464f6fb824 100644 --- a/src/platform/ESP32/BUILD.gn +++ b/src/platform/ESP32/BUILD.gn @@ -123,4 +123,6 @@ static_library("ESP32") { "ESP32DeviceInfoProvider.h", ] } + + cflags = [ "-Wconversion" ] } diff --git a/src/platform/ESP32/DiagnosticDataProviderImpl.cpp b/src/platform/ESP32/DiagnosticDataProviderImpl.cpp index e71a6a2386a954..4ac4bebca424ec 100644 --- a/src/platform/ESP32/DiagnosticDataProviderImpl.cpp +++ b/src/platform/ESP32/DiagnosticDataProviderImpl.cpp @@ -236,7 +236,18 @@ CHIP_ERROR DiagnosticDataProviderImpl::GetNetworkInterfaces(NetworkInterface ** ifp->Ipv4AddressSpans[0] = ByteSpan(ifp->Ipv4AddressesBuffer[0], kMaxIPv4AddrSize); ifp->IPv4Addresses = chip::app::DataModel::List(ifp->Ipv4AddressSpans, 1); } - ipv6_addr_count = esp_netif_get_all_ip6(ifa, ip6_addr); + + static_assert(kMaxIPv6AddrCount <= UINT8_MAX, "Count might not fit in ipv6_addr_count"); + static_assert(ArraySize(ip6_addr) >= LWIP_IPV6_NUM_ADDRESSES, "Not enough space for our addresses."); + auto addr_count = esp_netif_get_all_ip6(ifa, ip6_addr); + if (addr_count < 0) + { + ipv6_addr_count = 0; + } + else + { + ipv6_addr_count = static_cast(min(addr_count, static_cast(kMaxIPv6AddrCount))); + } for (uint8_t idx = 0; idx < ipv6_addr_count; ++idx) { memcpy(ifp->Ipv6AddressesBuffer[idx], ip6_addr[idx].addr, kMaxIPv6AddrSize); diff --git a/src/platform/ESP32/NetworkCommissioningDriver.cpp b/src/platform/ESP32/NetworkCommissioningDriver.cpp index c549b1b556d035..148704eeeeef2a 100644 --- a/src/platform/ESP32/NetworkCommissioningDriver.cpp +++ b/src/platform/ESP32/NetworkCommissioningDriver.cpp @@ -47,7 +47,9 @@ CHIP_ERROR GetConfiguredNetwork(Network & network) { return chip::DeviceLayer::Internal::ESP32Utils::MapError(err); } - uint8_t length = strnlen(reinterpret_cast(ap_info.ssid), DeviceLayer::Internal::kMaxWiFiSSIDLength); + static_assert(chip::DeviceLayer::Internal::kMaxWiFiSSIDLength <= UINT8_MAX, "SSID length might not fit in length"); + uint8_t length = + static_cast(strnlen(reinterpret_cast(ap_info.ssid), DeviceLayer::Internal::kMaxWiFiSSIDLength)); if (length > sizeof(network.networkID)) { return CHIP_ERROR_INTERNAL; @@ -75,8 +77,17 @@ CHIP_ERROR ESPWiFiDriver::Init(NetworkStatusChangeCallback * networkStatusChange { return CHIP_NO_ERROR; } - mSavedNetwork.credentialsLen = credentialsLen; - mSavedNetwork.ssidLen = ssidLen; + if (!CanCastTo(credentialsLen)) + { + return CHIP_ERROR_INCORRECT_STATE; + } + mSavedNetwork.credentialsLen = static_cast(credentialsLen); + + if (!CanCastTo(ssidLen)) + { + return CHIP_ERROR_INCORRECT_STATE; + } + mSavedNetwork.ssidLen = static_cast(ssidLen); mStagingNetwork = mSavedNetwork; mpScanCallback = nullptr; @@ -359,9 +370,14 @@ void ESPWiFiDriver::OnNetworkStatusChange() Status::kSuccess, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)), NullOptional); return; } + + // The disconnect reason for networking status changes is allowed to have + // manufacturer-specific values, which is why it's an int32_t, even though + // we just store a uint16_t value in it. + int32_t lastDisconnectReason = GetLastDisconnectReason(); mpStatusChangeCallback->OnNetworkingStatusChange( Status::kUnknownError, MakeOptional(ByteSpan(configuredNetwork.networkID, configuredNetwork.networkIDLen)), - MakeOptional(GetLastDisconnectReason())); + MakeOptional(lastDisconnectReason)); } void ESPWiFiDriver::ScanNetworks(ByteSpan ssid, WiFiDriver::ScanCallback * callback) @@ -386,7 +402,7 @@ CHIP_ERROR ESPWiFiDriver::SetLastDisconnectReason(const ChipDeviceEvent * event) return CHIP_NO_ERROR; } -int32_t ESPWiFiDriver::GetLastDisconnectReason() +uint16_t ESPWiFiDriver::GetLastDisconnectReason() { return mLastDisconnectedReason; } diff --git a/src/platform/ESP32/NetworkCommissioningDriver.h b/src/platform/ESP32/NetworkCommissioningDriver.h index d2c6bb160e745d..c6175938d76478 100644 --- a/src/platform/ESP32/NetworkCommissioningDriver.h +++ b/src/platform/ESP32/NetworkCommissioningDriver.h @@ -41,8 +41,9 @@ class ESPScanResponseIterator : public Iterator } item.security.SetRaw(mpScanResults[mIternum].authmode); - item.ssidLen = - strnlen(reinterpret_cast(mpScanResults[mIternum].ssid), chip::DeviceLayer::Internal::kMaxWiFiSSIDLength); + static_assert(chip::DeviceLayer::Internal::kMaxWiFiSSIDLength <= UINT8_MAX, "SSID length might not fit in item.ssidLen"); + item.ssidLen = static_cast( + strnlen(reinterpret_cast(mpScanResults[mIternum].ssid), chip::DeviceLayer::Internal::kMaxWiFiSSIDLength)); item.channel = mpScanResults[mIternum].primary; item.wiFiBand = chip::DeviceLayer::NetworkCommissioning::WiFiBand::k2g4; item.rssi = mpScanResults[mIternum].rssi; @@ -115,7 +116,7 @@ class ESPWiFiDriver final : public WiFiDriver void OnNetworkStatusChange(); CHIP_ERROR SetLastDisconnectReason(const ChipDeviceEvent * event); - int32_t GetLastDisconnectReason(); + uint16_t GetLastDisconnectReason(); static ESPWiFiDriver & GetInstance() { @@ -132,7 +133,7 @@ class ESPWiFiDriver final : public WiFiDriver ScanCallback * mpScanCallback; ConnectCallback * mpConnectCallback; NetworkStatusChangeCallback * mpStatusChangeCallback = nullptr; - int32_t mLastDisconnectedReason; + uint16_t mLastDisconnectedReason; }; } // namespace NetworkCommissioning diff --git a/src/platform/ESP32/route_hook/ESP32RouteHook.c b/src/platform/ESP32/route_hook/ESP32RouteHook.c index c3883c86856107..a63df118619ed3 100644 --- a/src/platform/ESP32/route_hook/ESP32RouteHook.c +++ b/src/platform/ESP32/route_hook/ESP32RouteHook.c @@ -57,12 +57,12 @@ static void ra_recv_handler(struct netif * netif, const uint8_t * icmp_payload, return; } icmp_payload += sizeof(struct ra_header); - payload_len -= sizeof(struct ra_header); + payload_len = (uint16_t)(payload_len - sizeof(struct ra_header)); while (payload_len >= 2) { uint8_t opt_type = icmp_payload[0]; - uint8_t opt_len = icmp_payload[1] << 3; + uint8_t opt_len = (uint8_t)(icmp_payload[1] << 3); if (opt_type == ND6_OPTION_TYPE_ROUTE_INFO && opt_len >= sizeof(route_option_t) - sizeof(ip6_addr_p_t) && !is_self_address(netif, src_addr) && payload_len >= opt_len) @@ -75,9 +75,9 @@ static void ra_recv_handler(struct netif * netif, const uint8_t * icmp_payload, { break; } - uint8_t prefix_len_bytes = (route_option.prefix_length + 7) / 8; - int8_t preference = -2 * ((route_option.preference >> 4) & 1) + (((route_option.preference) >> 3) & 1); - uint8_t rio_data_len = opt_len - sizeof(route_option) + sizeof(ip6_addr_p_t); + uint8_t prefix_len_bytes = (uint8_t)((route_option.prefix_length + 7) / 8); + int8_t preference = (int8_t)(-2 * ((route_option.preference >> 4) & 1) + (((route_option.preference) >> 3) & 1)); + uint8_t rio_data_len = (uint8_t)(opt_len - sizeof(route_option) + sizeof(ip6_addr_p_t)); ESP_LOGI(TAG, "Received RIO"); if (rio_data_len >= prefix_len_bytes) @@ -101,7 +101,7 @@ static void ra_recv_handler(struct netif * netif, const uint8_t * icmp_payload, } } icmp_payload += opt_len; - payload_len -= opt_len; + payload_len = (uint16_t)(payload_len - opt_len); } } @@ -136,7 +136,7 @@ static uint8_t icmp6_raw_recv_handler(void * arg, struct raw_pcb * pcb, struct p return 0; } - icmp_payload_len = p->tot_len - sizeof(struct ip6_hdr); + icmp_payload_len = (uint16_t)(p->tot_len - sizeof(struct ip6_hdr)); icmp_payload = p->payload + sizeof(struct ip6_hdr); icmp6_header = (struct icmp6_hdr *) icmp_payload; @@ -155,7 +155,12 @@ esp_err_t esp_route_hook_init(esp_netif_t * netif) esp_err_t ret = ESP_OK; ESP_RETURN_ON_FALSE(netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface"); - lwip_netif = netif_get_by_index(esp_netif_get_netif_impl_index(netif)); + int netif_idx = esp_netif_get_netif_impl_index(netif); + if (netif_idx < 0 || netif_idx > UINT8_MAX) + { + return ESP_ERR_INVALID_SIZE; + } + lwip_netif = netif_get_by_index((uint8_t) netif_idx); ESP_RETURN_ON_FALSE(lwip_netif != NULL, ESP_ERR_INVALID_ARG, TAG, "Invalid network interface"); for (esp_route_hook_t * iter = s_hooks; iter != NULL; iter++)