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

Bluetooth: GMAP: Replace busy bool with atomic #78127

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

Thalley
Copy link
Collaborator

@Thalley Thalley commented Sep 7, 2024

Replace the busy boolean flag with an atomic value. This prevents any race conditions with the GMAP client implementation.

Replace the busy boolean flag with an atomic value.
This prevents any race conditions with the GMAP client implementation.

Signed-off-by: Emil Gydesen <[email protected]>
Comment on lines -651 to +658
if (gmap_cli->busy) {
if (atomic_test_and_set_bit(gmap_cli->flags, GMAP_CLIENT_FLAG_BUSY)) {
Copy link
Member

Choose a reason for hiding this comment

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

Where was gmap_cli->busy getting set to true previously? I can't see you modify that place in this PR. You're adding the analogue to that here (set_bit()), but seems like the original bool was never actually set anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like you are correct. It was, incorrectly, never set to true it seems.

Arguably we should add BSIM tests for this behavior as well

@nashif nashif merged commit c012ece into zephyrproject-rtos:main Sep 11, 2024
32 checks passed
@Thalley Thalley deleted the gmap_atomic branch September 11, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants