-
Notifications
You must be signed in to change notification settings - Fork 8
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
make NMEA parser more robust #20
Comments
Closed
@linuxianer99 : I'll start working on this. Do you think I can do this in 1 commit and PR? |
bomilkar
added a commit
to bomilkar/variod
that referenced
this issue
May 23, 2020
It improves robustness of NMEA parsers. It presents one NMEA sentence at a time to be parsed. It follows the guidelines for NMEA sentence structure given here: https://en.wikipedia.org/wiki/NMEA_0183#Message_structure with listed exeptions: NMEA sentences are only valid if they have a checksum. The <CR><LF> at the end of the sentence is not enforced. One of the 2 is sufficient, but not less than 1 is required. The next steps: Implement the request to XCSoar to send current settings. Handle incomplete senteces from recv(). Both are marked in the code as TODO:
For the most part addressed with #30 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Both parse_NMEA_sensor() and parse_NMEA_command() can easily run into Segmentation Fault.
Look at the code and think what happens if you send parse_NMEA_sensor() a sentence (fraction) like this:
$POV,E,Q,E,Q
It will run into a Segmentation Fault when it reaches the end of the line.
Similarly for parse_NMEA_command() when it receives
CC,M
I think variod should avoid Segmentation Faults if at all possible.
A message like this
BLAH,E,1.1,Qjhsadjahdskhd,100.0,Puwezqez,*99
It will set TE to 1.1, dynamic pressure to 100.0 and Static to 0 ignoring the checksum error and the other garbage. I think we should apply reasonable checks before accepting sentences.
Then eliminate the memory leak. I count 6 malloc() and not a single free().
In the process of fixing the above it may turn out to be best to let the 2 parser functions work on one sentence at a time.
The text was updated successfully, but these errors were encountered: