Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WiFiGeneric CrashFix #8028

Conversation

mrengineer7777
Copy link
Collaborator

@mrengineer7777 mrengineer7777 commented Apr 4, 2023

Description of Change

This PR resolves crashes in WiFiGeneric.cpp. The crashes can occur with CORE_DEBUG_LEVEL >= 2.

Observed:

  • Crash due to bad index into system_event_reasons[].

Potential:

  • Crash on bad index into arduino_event_names[]. e.g. New entry added but string array not updated.
  • Crash if event is null.

Changelog:

  • Replaced system_event_reasons[] and reason2str() with WiFiErrReasonToCStr().
  • Replaced arduino_event_names[] with ArduinoEventIdToCStr().
  • Return if event is null.

Advantages:

  • Easier to maintain string conversions when types are updated.
  • Users can call ArduinoEventIdToCStr() and WiFiErrReasonToCStr() for their own use.
  • Codespace is only used if the function is called.
  • Functions return blank strings rather than crashing when an unknown type is encountered.
  • WiFiErrReasonToCStr() works with multiple Arduino versions by using conditional compiles.

Observations:

  • Several additions on wifi_err_reason_t.
  • No changes on arduino_event_id_t.

Tests scenarios

Tested on custom hardware (ESP32-WROOM-32UE module) with arduino-esp32 v2.0.7. Works on both WiFi and Ethernet events. No crashes observed so far.

Related links

Closes #7959

@mrengineer7777
Copy link
Collaborator Author

Retested WIFI and ETH events with latest PR changes. Ready for review.

@VojtechBartoska VojtechBartoska added the Area: WiFi Issue related to WiFi label Apr 5, 2023
@VojtechBartoska VojtechBartoska added the Status: Review needed Issue or PR is awaiting review label Apr 5, 2023
@VojtechBartoska VojtechBartoska added this to the 2.0.8 milestone Apr 5, 2023
@mrengineer7777
Copy link
Collaborator Author

Rebased on latest master. Why is Github now showing an unrelated file modified?

Fixes crash in WiFiGeneric due to bad index into system_event_reasons[].  Also replaced arduino_event_names[] to prevent a potential crash.
Supports multiple Arduino versions.
@mrengineer7777
Copy link
Collaborator Author

Fixed the rebase regression with Github Desktop client. I'll let the devs figure out how to sync with master.

@mrengineer7777
Copy link
Collaborator Author

Replaced by #8043 , #8044

@mrengineer7777 mrengineer7777 deleted the WFG_EventCallback_CrashFix branch April 28, 2023 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: WiFi Issue related to WiFi Status: Review needed Issue or PR is awaiting review
Projects
Development

Successfully merging this pull request may close these issues.

WiFiGeneric Crash on WIFI_REASON_UNSPECIFIED
2 participants