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

xQueuePeek with third parameter '0' does not initialize the 2nd param… #501

Merged
merged 1 commit into from
Nov 7, 2019

Conversation

markhermelinggt
Copy link
Contributor

…eter

If the queue is empty, then priority in commanderSetSetpoint is not initialized. The same holds for commanderGetSetpoint (the XQueuePeek would not initialize set point on line 81 if the queue is empty, but it is not clear to me how to fix that code.

@ataffanel : This is a similar problem as I reported earlier. Unclear to me if this queue can ever be empty.

…eter

If the queue is empty, then priority in commanderSetSetpoint is not initialized. The same holds for commanderGetSetpoint (the XQueuePeek would not initialize set point on line 81 if the queue is empty, but it is not clear to me how to fix that code.

@ataffanel : This is a similar problem as I reported earlier. Unclear to me if this queue can ever be empty.
@krichardsson
Copy link
Contributor

Hi!

This queue should never be empty since an element is added in the commanderInit() and elements never are removed. However the queue is currently not defined as static, which it should be to make sure no other code outside the file is tampering with it, so this is not obvious.
It is probably a good idea to check the return value of the peek call, but since the queue should never be empty (by design), it might be better to check the return value using an assert to indicate that this is an unexpected event and fail fast, as opposed to handle the error.

I will merge the pull request, but make some modifications in a second commit.

Thanks for finding dodgy code and your effort!

@krichardsson krichardsson merged commit 24590ec into bitcraze:master Nov 7, 2019
krichardsson added a commit that referenced this pull request Nov 7, 2019
@krichardsson krichardsson added this to the next-release milestone Nov 7, 2019
cafeciaojoe pushed a commit to cafeciaojoe/crazyflie-firmware that referenced this pull request Sep 27, 2024
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.

2 participants