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

Restore null pointer detection in parser. #169

Merged
merged 3 commits into from
Jul 26, 2018

Conversation

rolandash
Copy link
Contributor

Removal of null pointer detection in v2.0 suppressed conveniency invocation to parser like:

mavlink_parse_char(MAVLINK_BACKHAUL, *tp, NULL, NULL);

Better to restore.

@amilcarlucas amilcarlucas requested a review from tridge April 11, 2018 13:18
@peterbarker
Copy link
Contributor

@rolandash Why would you want to invoke mavlink_parse_char like that?

@rolandash
Copy link
Contributor Author

rolandash commented Apr 12, 2018

The library uses internal rx buffers by default, so that developers do not need to allocate rx buffers and maintain the pointers to the buffer/status by themselves, actually they have no reason to care about these pointers at all, if they choose to use internal buffers.
I think this is the original design purpose, as this can help to generate simpler code in application side.

With the null pointer detections been removed, it either forces developer to use external buffers, or have to get internal buffer pointers first explicitly which are intended to hide from them, and then pass them to the parser which already know these pointers quite well, this seems rather odd in logic, and also deviated from the original design purpose.

r_message->len = rxmsg->len; // Provide visibility on how far we are into current msg
}
if (NULL != r_mavlink_status) {
r_mavlink_status->parse_state = status->parse_state;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent, one whitespace too much

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @amilcarlucas
now the indent should look right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants