Fixed nullptr crash in ETH IP comparing code - #5796
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe Ethernet static IP configuration check now requires a ChangesEthernet static IP configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@Tarik2142 can you explain a bit more on which build this crash is happening?
The cast to (uint32_t) was added to enforce that the uint32_t overload of "==" gets used, this overload should also handle nullptr properly - at least in the new framework. |
|
|
||
| // https://github.com/wled/WLED/issues/5247 | ||
| if (multiWiFi[0].staticIP != (uint32_t)0x00000000 && multiWiFi[0].staticGW != (uint32_t)0x00000000) { | ||
| if (multiWiFi.size() && multiWiFi[0].staticIP != IPAddress() && multiWiFi[0].staticGW != IPAddress()) { |
There was a problem hiding this comment.
It might be that the addition of if (multiWiFi.size() is the actual fix.
@coderabbitai how is IPAddress() defined? Is it the same as 0.0.0.0 ?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '=== IPAddress declarations/uses ==='
rg -n -C 3 'class IPAddress|IPAddress::IPAddress|IPAddress\(\)' \
-g '*.{h,hpp,cpp,ino}' .
printf '%s\n' '=== Arduino-ESP32 framework version references ==='
rg -n -C 2 'arduino-esp32|ESP_ARDUINO_VERSION|platform.*espressif32' \
-g 'platformio.ini' -g 'platformio*.ini' -g 'package.json' -g '*.json' -g '*.md' .Length of output: 14902
@softhack007, yes. In Arduino-ESP32, a default-constructed IPAddress() has all four IPv4 octets set to zero. It represents 0.0.0.0.
The two changes address different conditions:
multiWiFi.size()prevents access tomultiWiFi[0]when the vector is empty. This can be the crash fix if no Wi-Fi entry exists.staticIP != IPAddress()andstaticGW != IPAddress()compare twoIPAddressobjects. This selects theIPAddresscomparison overload.
The old (uint32_t)0x00000000 value can convert to a null pointer. It then selects the const uint8_t * comparison overload shown in the Comment image. That overload reads the pointer as address bytes, so it can dereference nullptr.
Therefore, IPAddress() is the correct explicit test for 0.0.0.0, and multiWiFi.size() is the required bounds guard. Both changes are valid.
🐇🔍
You are interacting with an AI system.
I am working on a fork for ESP32-S3 + W5500 ethernet https://github.com/smlight-tech/A1-SLWF-09-WLED I can say for sure that |

This PR fixes a nullptr crash when using ethernet.

The current implementation treats the passed value as a pointer, and since it is 0, it causes a crash.
Сomparing with
IPAddress()is a more reliable option.Summary by CodeRabbit