From 9b4bebbdf3bc94c87b4aa9bb05d368cf304354a4 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Sat, 24 May 2025 21:20:36 +0200 Subject: [PATCH] Bluetooth: ASCS: Sonarcloud fixes Made a few complex functions simpler Added missing default cases in switches Fixes a bad cast that removed const Moved loop iterators to inner loop Signed-off-by: Emil Gydesen --- subsys/bluetooth/audio/ascs.c | 179 +++++++++++-------------- subsys/bluetooth/audio/ascs_internal.h | 12 +- subsys/bluetooth/audio/cap_acceptor.c | 26 ++++ subsys/bluetooth/audio/cap_internal.h | 2 + 4 files changed, 110 insertions(+), 109 deletions(-) diff --git a/subsys/bluetooth/audio/ascs.c b/subsys/bluetooth/audio/ascs.c index 0d73444dcd1..65acb0507e8 100644 --- a/subsys/bluetooth/audio/ascs.c +++ b/subsys/bluetooth/audio/ascs.c @@ -689,6 +689,9 @@ int ascs_ep_set_state(struct bt_bap_ep *ep, uint8_t state) default: break; } break; + default: + __ASSERT_PRINT("Unhandled state %d", state); + break; } if (!valid_state_transition) { @@ -1191,7 +1194,7 @@ static int ase_release(struct bt_ascs_ase *ase, uint8_t reason, struct bt_bap_as } err = unicast_server_cb->release(ase->ep.stream, rsp); - if (err) { + if (err != 0) { if (rsp->code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { *rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, BT_BAP_ASCS_REASON_NONE); @@ -1254,7 +1257,7 @@ static int ase_disable(struct bt_ascs_ase *ase, uint8_t reason, struct bt_bap_as } err = unicast_server_cb->disable(stream, rsp); - if (err) { + if (err != 0) { if (rsp->code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { *rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, BT_BAP_ASCS_REASON_NONE); @@ -1488,7 +1491,7 @@ static void ascs_cp_cfg_changed(const struct bt_gatt_attr *attr, uint16_t value) } static int ascs_ep_set_codec(struct bt_bap_ep *ep, uint8_t id, uint16_t cid, uint16_t vid, - uint8_t *cc, uint8_t len, struct bt_bap_ascs_rsp *rsp) + const uint8_t *cc, uint8_t len, struct bt_bap_ascs_rsp *rsp) { const struct bt_audio_codec_cap *codec_cap; struct bt_audio_codec_cfg *codec_cfg; @@ -1524,7 +1527,7 @@ static int ascs_ep_set_codec(struct bt_bap_ep *ep, uint8_t id, uint16_t cid, uin codec_cfg->cid = cid; codec_cfg->vid = vid; codec_cfg->data_len = len; - memcpy(codec_cfg->data, cc, len); + (void)memcpy(codec_cfg->data, cc, len); codec_cfg->path_id = codec_cap->path_id; *rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_SUCCESS, BT_BAP_ASCS_REASON_NONE); @@ -1545,16 +1548,14 @@ static int ase_config(struct bt_ascs_ase *ase, const struct bt_ascs_config *cfg) ase, cfg->latency, cfg->phy, cfg->codec.id, cfg->codec.cid, cfg->codec.vid, cfg->cc_len); - if (cfg->latency < BT_ASCS_CONFIG_LATENCY_LOW || - cfg->latency > BT_ASCS_CONFIG_LATENCY_HIGH) { + if (!IN_RANGE(cfg->latency, BT_ASCS_CONFIG_LATENCY_LOW, BT_ASCS_CONFIG_LATENCY_HIGH)) { LOG_WRN("Invalid latency: 0x%02x", cfg->latency); ascs_cp_rsp_add(ASE_ID(ase), BT_BAP_ASCS_RSP_CODE_CONF_INVALID, BT_BAP_ASCS_REASON_LATENCY); return -EINVAL; } - if (cfg->phy < BT_ASCS_CONFIG_PHY_LE_1M || - cfg->phy > BT_ASCS_CONFIG_PHY_LE_CODED) { + if (!IN_RANGE(cfg->phy, BT_ASCS_CONFIG_PHY_LE_1M, BT_ASCS_CONFIG_PHY_LE_CODED)) { LOG_WRN("Invalid PHY: 0x%02x", cfg->phy); ascs_cp_rsp_add(ASE_ID(ase), BT_BAP_ASCS_RSP_CODE_CONF_INVALID, BT_BAP_ASCS_REASON_PHY); @@ -1589,18 +1590,20 @@ static int ase_config(struct bt_ascs_ase *ase, const struct bt_ascs_config *cfg) (void)memcpy(&codec_cfg, &ase->ep.codec_cfg, sizeof(codec_cfg)); err = ascs_ep_set_codec(&ase->ep, cfg->codec.id, sys_le16_to_cpu(cfg->codec.cid), - sys_le16_to_cpu(cfg->codec.vid), (uint8_t *)cfg->cc, cfg->cc_len, - &rsp); - if (err) { + sys_le16_to_cpu(cfg->codec.vid), (const uint8_t *)cfg->cc, + cfg->cc_len, &rsp); + if (err != 0) { ascs_app_rsp_warn_valid(&rsp); (void)memcpy(&ase->ep.codec_cfg, &codec_cfg, sizeof(codec_cfg)); ascs_cp_rsp_add(ASE_ID(ase), rsp.code, rsp.reason); return err; } - if (ase->ep.stream != NULL) { - if (unicast_server_cb != NULL && - unicast_server_cb->reconfig != NULL) { + if (unicast_server_cb == NULL) { + err = -ENOTSUP; + rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, BT_BAP_ASCS_REASON_NONE); + } else if (ase->ep.stream != NULL) { + if (unicast_server_cb->reconfig != NULL) { err = unicast_server_cb->reconfig(ase->ep.stream, ase->ep.dir, &ase->ep.codec_cfg, &ase->ep.qos_pref, &rsp); @@ -1610,38 +1613,23 @@ static int ase_config(struct bt_ascs_ase *ase, const struct bt_ascs_config *cfg) BT_BAP_ASCS_REASON_NONE); } - if (err == 0 && !bt_bap_valid_qos_pref(&ase->ep.qos_pref)) { - LOG_ERR("Invalid QoS preferences"); + if (err == 0) { + if (!bt_bap_valid_qos_pref(&ase->ep.qos_pref)) { + LOG_DBG("Invalid QoS preferences"); - /* If the upper layers provide an invalid QoS preferences we reject the - * request from the client, as it would not be able to parse the result - * as valid anyways - */ - err = -EINVAL; - } - - if (err) { - ascs_app_rsp_warn_valid(&rsp); - - if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { - rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, - BT_BAP_ASCS_REASON_NONE); + /* If the upper layers provide an invalid QoS preferences we reject + * the request from the client, as it would not be able to parse the + * result as valid anyways + */ + err = -EINVAL; } - LOG_ERR("Reconfig failed: err %d, code %u, reason %u", - err, rsp.code, rsp.reason); - - (void)memcpy(&ase->ep.codec_cfg, &codec_cfg, sizeof(codec_cfg)); - ascs_cp_rsp_add(ASE_ID(ase), rsp.code, rsp.reason); - - return err; + stream = ase->ep.stream; + ascs_app_rsp_warn_valid(&rsp); } - - stream = ase->ep.stream; } else { stream = NULL; - if (unicast_server_cb != NULL && - unicast_server_cb->config != NULL) { + if (unicast_server_cb->config != NULL) { err = unicast_server_cb->config(ase->conn, &ase->ep, ase->ep.dir, &ase->ep.codec_cfg, &stream, &ase->ep.qos_pref, &rsp); @@ -1651,34 +1639,41 @@ static int ase_config(struct bt_ascs_ase *ase, const struct bt_ascs_config *cfg) BT_BAP_ASCS_REASON_NONE); } - if (err == 0 && !bt_bap_valid_qos_pref(&ase->ep.qos_pref)) { - LOG_ERR("Invalid QoS preferences"); - - /* If the upper layers provide an invalid QoS preferences we reject the - * request from the client, as it would not be able to parse the result - * as valid anyways - */ - err = -EINVAL; + if (err == 0 && stream == NULL) { + LOG_DBG("Upper layer did not provide a stream object"); + err = -ENOMEM; } - if (err || stream == NULL) { - ascs_app_rsp_warn_valid(&rsp); + if (err == 0 && stream != NULL) { + if (!bt_bap_valid_qos_pref(&ase->ep.qos_pref)) { + LOG_DBG("Invalid QoS preferences"); - if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { - rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, - BT_BAP_ASCS_REASON_NONE); + /* If the upper layers provide an invalid QoS preferences we reject + * the request from the client, as it would not be able to parse the + * result as valid anyways + */ + err = -EINVAL; } - LOG_ERR("Config failed: err %d, stream %p, code %u, reason %u", - err, stream, rsp.code, rsp.reason); + bt_bap_stream_init(stream); + } + } - (void)memcpy(&ase->ep.codec_cfg, &codec_cfg, sizeof(codec_cfg)); - ascs_cp_rsp_add(ASE_ID(ase), rsp.code, rsp.reason); + if (err != 0) { + ascs_app_rsp_warn_valid(&rsp); - return err ? err : -ENOMEM; + if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { + rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, + BT_BAP_ASCS_REASON_NONE); } - bt_bap_stream_init(stream); + LOG_DBG("Config failed: err %d, stream %p, code %u, reason %u", err, stream, + rsp.code, rsp.reason); + + (void)memcpy(&ase->ep.codec_cfg, &codec_cfg, sizeof(codec_cfg)); + ascs_cp_rsp_add(ASE_ID(ase), rsp.code, rsp.reason); + + return err; } ascs_cp_rsp_success(ASE_ID(ase)); @@ -1962,9 +1957,9 @@ static void ase_qos(struct bt_ascs_ase *ase, uint8_t cig_id, uint8_t cis_id, } if (unicast_server_cb != NULL && unicast_server_cb->qos != NULL) { - int err = unicast_server_cb->qos(stream, qos, rsp); + const int err = unicast_server_cb->qos(stream, qos, rsp); - if (err) { + if (err != 0) { if (rsp->code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { *rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, BT_BAP_ASCS_REASON_NONE); @@ -2121,7 +2116,6 @@ static bool is_context_available(struct bt_conn *conn, enum bt_audio_dir dir, ui static bool ascs_parse_metadata(struct bt_data *data, void *user_data) { struct ascs_parse_result *result = user_data; - const struct bt_bap_ep *ep = result->ep; const uint8_t data_len = data->data_len; const uint8_t data_type = data->type; const uint8_t *data_value = data->data; @@ -2156,14 +2150,13 @@ static bool ascs_parse_metadata(struct bt_data *data, void *user_data) /* The CAP acceptor shall not accept metadata with unsupported stream context. */ if (IS_ENABLED(CONFIG_BT_CAP_ACCEPTOR) && - data_type == BT_AUDIO_METADATA_TYPE_STREAM_CONTEXT) { - if (!is_context_available(result->conn, ep->dir, context)) { - LOG_WRN("Context 0x%04x is unavailable", context); - *result->rsp = BT_BAP_ASCS_RSP( - BT_BAP_ASCS_RSP_CODE_METADATA_REJECTED, data_type); - result->err = -EACCES; - return false; - } + data_type == BT_AUDIO_METADATA_TYPE_STREAM_CONTEXT && + !is_context_available(result->conn, result->ep->dir, context)) { + LOG_WRN("Context 0x%04x is unavailable", context); + *result->rsp = + BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_METADATA_REJECTED, data_type); + result->err = -EACCES; + return false; } break; @@ -2179,24 +2172,9 @@ static bool ascs_parse_metadata(struct bt_data *data, void *user_data) break; case BT_AUDIO_METADATA_TYPE_CCID_LIST: { /* Verify that the CCID is a known CCID on the writing device */ - if (IS_ENABLED(CONFIG_BT_CAP_ACCEPTOR)) { - for (uint8_t i = 0; i < data_len; i++) { - const uint8_t ccid = data_value[i]; - - if (!bt_cap_acceptor_ccid_exist(ep->stream->conn, ccid)) { - LOG_WRN("CCID %u is unknown", ccid); - - /* TBD: - * Should we reject the Metadata? - * - * Should unknown CCIDs trigger a - * discovery procedure for TBS or MCS? - * - * Or should we just accept as is, and - * then let the application decide? - */ - } - } + if (IS_ENABLED(CONFIG_BT_CAP_ACCEPTOR) && + !bt_cap_acceptor_ccids_exist(result->ep->stream->conn, data_value, data_len)) { + LOG_WRN("Metadata contain unknown CCID(s)"); } break; @@ -2317,7 +2295,7 @@ static void ase_metadata(struct bt_ascs_ase *ase, struct bt_ascs_metadata *meta) BT_BAP_ASCS_REASON_NONE); } - if (err) { + if (err != 0) { ascs_app_rsp_warn_valid(&rsp); if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { @@ -2374,7 +2352,7 @@ static int ase_enable(struct bt_ascs_ase *ase, struct bt_ascs_metadata *meta) BT_BAP_ASCS_REASON_NONE); } - if (err) { + if (err != 0) { ascs_app_rsp_warn_valid(&rsp); if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { @@ -2446,7 +2424,6 @@ static ssize_t ascs_enable(struct bt_conn *conn, struct net_buf_simple *buf) { const struct bt_ascs_enable_op *req; struct bt_ascs_metadata *meta; - int i; if (!is_valid_enable_len(conn, buf)) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); @@ -2456,7 +2433,7 @@ static ssize_t ascs_enable(struct bt_conn *conn, struct net_buf_simple *buf) LOG_DBG("num_ases %u", req->num_ases); - for (i = 0; i < req->num_ases; i++) { + for (uint8_t i = 0U; i < req->num_ases; i++) { struct bt_ascs_ase *ase; meta = net_buf_simple_pull_mem(buf, sizeof(*meta)); @@ -2521,7 +2498,7 @@ static void ase_start(struct bt_ascs_ase *ase) BT_BAP_ASCS_REASON_NONE); } - if (err) { + if (err != 0) { ascs_app_rsp_warn_valid(&rsp); if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { @@ -2572,7 +2549,6 @@ static bool is_valid_start_len(struct bt_conn *conn, struct net_buf_simple *buf) static ssize_t ascs_start(struct bt_conn *conn, struct net_buf_simple *buf) { const struct bt_ascs_start_op *req; - int i; if (!is_valid_start_len(conn, buf)) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); @@ -2582,7 +2558,7 @@ static ssize_t ascs_start(struct bt_conn *conn, struct net_buf_simple *buf) LOG_DBG("num_ases %u", req->num_ases); - for (i = 0; i < req->num_ases; i++) { + for (uint8_t i = 0U; i < req->num_ases; i++) { struct bt_ascs_ase *ase; uint8_t id; @@ -2723,7 +2699,7 @@ static void ase_stop(struct bt_ascs_ase *ase) BT_BAP_ASCS_REASON_NONE); } - if (err) { + if (err != 0) { ascs_app_rsp_warn_valid(&rsp); if (rsp.code == BT_BAP_ASCS_RSP_CODE_SUCCESS) { @@ -2784,7 +2760,6 @@ static bool is_valid_stop_len(struct bt_conn *conn, struct net_buf_simple *buf) static ssize_t ascs_stop(struct bt_conn *conn, struct net_buf_simple *buf) { const struct bt_ascs_start_op *req; - int i; if (!is_valid_stop_len(conn, buf)) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); @@ -2794,7 +2769,7 @@ static ssize_t ascs_stop(struct bt_conn *conn, struct net_buf_simple *buf) LOG_DBG("num_ases %u", req->num_ases); - for (i = 0; i < req->num_ases; i++) { + for (uint8_t i = 0U; i < req->num_ases; i++) { struct bt_ascs_ase *ase; uint8_t id; @@ -2884,7 +2859,6 @@ static ssize_t ascs_metadata(struct bt_conn *conn, struct net_buf_simple *buf) { const struct bt_ascs_metadata_op *req; struct bt_ascs_metadata *meta; - int i; if (!is_valid_metadata_len(conn, buf)) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); @@ -2894,7 +2868,7 @@ static ssize_t ascs_metadata(struct bt_conn *conn, struct net_buf_simple *buf) LOG_DBG("num_ases %u", req->num_ases); - for (i = 0; i < req->num_ases; i++) { + for (uint8_t i = 0U; i < req->num_ases; i++) { struct bt_ascs_ase *ase; meta = net_buf_simple_pull_mem(buf, sizeof(*meta)); @@ -2959,7 +2933,6 @@ static bool is_valid_release_len(struct bt_conn *conn, struct net_buf_simple *bu static ssize_t ascs_release(struct bt_conn *conn, struct net_buf_simple *buf) { const struct bt_ascs_release_op *req; - int i; if (!is_valid_release_len(conn, buf)) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); @@ -2969,7 +2942,7 @@ static ssize_t ascs_release(struct bt_conn *conn, struct net_buf_simple *buf) LOG_DBG("num_ases %u", req->num_ases); - for (i = 0; i < req->num_ases; i++) { + for (uint8_t i = 0U; i < req->num_ases; i++) { struct bt_bap_ascs_rsp rsp = BT_BAP_ASCS_RSP(BT_BAP_ASCS_RSP_CODE_UNSPECIFIED, BT_BAP_ASCS_REASON_NONE); struct bt_ascs_ase *ase; @@ -3220,7 +3193,7 @@ int bt_ascs_init(const struct bt_bap_unicast_server_cb *cb) } err = bt_iso_server_register(&iso_server); - if (err) { + if (err != 0) { LOG_ERR("Failed to register ISO server %d", err); return err; } diff --git a/subsys/bluetooth/audio/ascs_internal.h b/subsys/bluetooth/audio/ascs_internal.h index 97658999cc6..1a109f32eb2 100644 --- a/subsys/bluetooth/audio/ascs_internal.h +++ b/subsys/bluetooth/audio/ascs_internal.h @@ -274,9 +274,9 @@ static inline const char *bt_ascs_op_str(uint8_t op) return "Update Metadata"; case BT_ASCS_RELEASE_OP: return "Release"; + default: + return "Unknown"; } - - return "Unknown"; } static inline const char *bt_ascs_rsp_str(uint8_t code) @@ -312,9 +312,9 @@ static inline const char *bt_ascs_rsp_str(uint8_t code) return "Insufficient Resources"; case BT_BAP_ASCS_RSP_CODE_UNSPECIFIED: return "Unspecified Error"; + default: + return "Unknown"; } - - return "Unknown"; } static inline const char *bt_ascs_reason_str(uint8_t reason) @@ -342,9 +342,9 @@ static inline const char *bt_ascs_reason_str(uint8_t reason) return "Presentation Delay"; case BT_BAP_ASCS_REASON_CIS: return "Invalid ASE CIS Mapping"; + default: + return "Unknown"; } - - return "Unknown"; } int bt_ascs_init(const struct bt_bap_unicast_server_cb *cb); diff --git a/subsys/bluetooth/audio/cap_acceptor.c b/subsys/bluetooth/audio/cap_acceptor.c index f8365eccade..151f60b5d1e 100644 --- a/subsys/bluetooth/audio/cap_acceptor.c +++ b/subsys/bluetooth/audio/cap_acceptor.c @@ -94,3 +94,29 @@ bool bt_cap_acceptor_ccid_exist(const struct bt_conn *conn, uint8_t ccid) return false; } + +bool bt_cap_acceptor_ccids_exist(const struct bt_conn *conn, const uint8_t ccids[], + uint8_t ccid_cnt) +{ + for (uint8_t i = 0; i < ccid_cnt; i++) { + const uint8_t ccid = ccids[i]; + + if (!bt_cap_acceptor_ccid_exist(conn, ccid)) { + LOG_DBG("CCID %u is unknown", ccid); + + /* TBD: + * Should we reject the Metadata? + * + * Should unknown CCIDs trigger a + * discovery procedure for TBS or MCS? + * + * Or should we just accept as is, and + * then let the application decide? + */ + return false; + } + } + + /* This will also return true if the ccid_cnt is 0 which is intended */ + return true; +} diff --git a/subsys/bluetooth/audio/cap_internal.h b/subsys/bluetooth/audio/cap_internal.h index f6bdb40d6e3..ac9938ecd66 100644 --- a/subsys/bluetooth/audio/cap_internal.h +++ b/subsys/bluetooth/audio/cap_internal.h @@ -24,6 +24,8 @@ #include bool bt_cap_acceptor_ccid_exist(const struct bt_conn *conn, uint8_t ccid); +bool bt_cap_acceptor_ccids_exist(const struct bt_conn *conn, const uint8_t ccids[], + uint8_t ccid_cnt); void bt_cap_initiator_codec_configured(struct bt_cap_stream *cap_stream); void bt_cap_initiator_qos_configured(struct bt_cap_stream *cap_stream);