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.

Sign up for GitHub

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: tester: audio: Add NULL checks #82530

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+17 −2
Open

bluetooth: tester: audio: Add NULL checks #82530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion tests/bluetooth/tester/src/audio/btp_cap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,11 @@ static uint8_t btp_cap_broadcast_source_release(const void *cmd, uint16_t cmd_le

LOG_DBG("");

/* If no source has been created yet, there is nothing to release */
if (source == NULL || source->cap_broadcast == NULL) {
return BTP_STATUS_SUCCESS;
}
Comment on lines +700 to +703
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sjanc I think this is a missing part of the existing BTP documentation. The documentation only contain information about what the command and responses contain, but unlike e.g. the HCI specification, it does not describe the expected behavior of commands in error cases.

For example it seems that BlueKitchen will return success when removing or stopping a non-existing source, where Zephyr (until this PR) would return an error.

I think that if we want BTP to be a proper protocol, we need to start expanding with additional status codes as well as expected behavior, otherwise it's basically a guessing game for the DUT when implementing it if the autopts client expects undocumented behavior.


err = bt_cap_initiator_broadcast_audio_delete(source->cap_broadcast);
if (err != 0) {
LOG_DBG("Unable to delete broadcast source: %d", err);
Expand Down Expand Up @@ -744,7 +749,11 @@ static uint8_t btp_cap_broadcast_adv_stop(const void *cmd, uint16_t cmd_len,
LOG_DBG("");

err = tester_gap_padv_stop();
if (err != 0) {
if (err == -ESRCH) {
/* Ext adv hasn't been created yet */
return BTP_STATUS_SUCCESS;
} else if (err != 0) {
LOG_DBG("Failed to stop periodic adv, err: %d", err);
return BTP_STATUS_FAILED;
}

Expand Down Expand Up @@ -786,6 +795,11 @@ static uint8_t btp_cap_broadcast_source_stop(const void *cmd, uint16_t cmd_len,
struct btp_bap_broadcast_local_source *source =
btp_bap_broadcast_local_source_get(cp->source_id);

/* If no source has been started yet, there is nothing to stop */
if (source == NULL || source->cap_broadcast == NULL) {
return BTP_STATUS_SUCCESS;
}

err = bt_cap_initiator_broadcast_audio_stop(source->cap_broadcast);
if (err != 0) {
LOG_ERR("Failed to stop audio source: %d", err);
Expand Down
3 changes: 2 additions & 1 deletion tests/bluetooth/tester/src/btp_gap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1548,7 +1548,8 @@ int tester_gap_padv_stop(void)
int err;

if (ext_adv == NULL) {
return -EINVAL;
/* Ext adv not yet created */
return -ESRCH;
}

/* Enable Periodic Advertising */
Expand Down
Loading