Fix _parseCSV() array index bug: combined PR #180 and PR #181 - #182
Merged
Merged
Conversation
parse_csv() advances bptr and strdups into buf on every comma, but on the terminating '\0' it only sets fEnd and breaks out of the loop. The content accumulated in tmp since the last comma was never emitted, so for an N-field CSV the function returned N-1 strings followed by NULL even though count_fields() correctly reported N. In practice this meant buf[N-1] was NULL. Callers like _parseCSV() that read the last field (e.g. atof(fields[3]) for the elevation in a value,lat,lon,ele location CSV) dereferenced NULL and crashed with LoadProhibited on ESP32. The bug was masked on main by a second bug in _parseCSV() that incorrectly read fields[1] three times instead of fields[1..3], so the last slot was never actually accessed. Fix: emit the final field from inside the '\0' case before setting fEnd. Uses the same strdup + allocation-failure cleanup as the ',' case.
In _parseCSV(), all three location fields (lat, lon, ele) were reading from fields[1], meaning lon and ele always returned the latitude value instead of their own data. Also removes an unreachable return statement at the end of the function.
brentru
approved these changes
Apr 14, 2026
brentru
left a comment
brentru
left a comment
Member
There was a problem hiding this comment.
@mikeysklar Please update library.properties with a version bump.
The updates to the code look OK. I have tested on the Feather ESP8266.
Member
|
Thanks, will merge and release when tests complete |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two bugs in the location CSV parsing that mask each other and together cause a crash on every received location message.
#181 —
parse_csv()silently dropped the last field of every CSV string, leavingNULLin the final slot instead of the elevation value.#180 —
_parseCSV()readfields[1]three times instead offields[1],fields[2],fields[3], solonandelealways returned thelatvalue.Tested and verified on Adafruit Feather ESP32 V2, QT Py ESP32-S3, and Feather ESP8266.