From 3bc17d1d18d778ee90ebd98a8001c5384502bb28 Mon Sep 17 00:00:00 2001 From: Pavel Vasilyev Date: Fri, 17 Nov 2023 10:27:22 +0100 Subject: [PATCH] Bluetooth: Mesh: Fix async behavior of Private Beacon Client API The Private Beacon Client API requires a response argument to allow to call the API in the asynchronous manner (https://github.com/zephyrproject-rtos/zephyr/pull/56426). Because the removal of the EXPERIMENTAL tag for this API was not released yet, it should be OK to change this API. The EXPERIMENTAL tag has been removed here: https://github.com/zephyrproject-rtos/zephyr/pull/64866 Coverity-CID: 330039 Coverity-CID: 330029 Coverity-CID: 329977 Fixes #65336 Fixes #65338 Fixes #65354 Signed-off-by: Pavel Vasilyev --- .../zephyr/bluetooth/mesh/priv_beacon_cli.h | 31 ++++++++++++++----- subsys/bluetooth/mesh/priv_beacon_cli.c | 29 +++++++++-------- subsys/bluetooth/mesh/shell/priv_beacon.c | 6 ++-- tests/bluetooth/tester/src/btp_mesh.c | 6 ++-- tests/bsim/bluetooth/mesh/src/test_beacon.c | 14 ++++----- .../bluetooth/mesh/src/test_persistence.c | 6 ++-- 6 files changed, 56 insertions(+), 36 deletions(-) diff --git a/include/zephyr/bluetooth/mesh/priv_beacon_cli.h b/include/zephyr/bluetooth/mesh/priv_beacon_cli.h index 24fba6f67be..ac77b45839f 100644 --- a/include/zephyr/bluetooth/mesh/priv_beacon_cli.h +++ b/include/zephyr/bluetooth/mesh/priv_beacon_cli.h @@ -101,14 +101,20 @@ struct bt_mesh_priv_beacon_cli { /** @brief Set the target's Private Beacon state. * + * This method can be used asynchronously by setting @p rsp as NULL. + * This way the method will not wait for response and will return + * immediately after sending the command. + * @param net_idx Network index to encrypt with. * @param addr Target node address. - * @param val New Private Beacon value. Returns response status on success. + * @param val New Private Beacon value. + * @param rsp If set, returns response status on success. * * @return 0 on success, or (negative) error code otherwise. */ int bt_mesh_priv_beacon_cli_set(uint16_t net_idx, uint16_t addr, - struct bt_mesh_priv_beacon *val); + struct bt_mesh_priv_beacon *val, + struct bt_mesh_priv_beacon *rsp); /** @brief Get the target's Private Beacon state. * @@ -122,16 +128,20 @@ int bt_mesh_priv_beacon_cli_get(uint16_t net_idx, uint16_t addr, struct bt_mesh_priv_beacon *val); /** @brief Set the target's Private GATT Proxy state. + * + * This method can be used asynchronously by setting @p rsp as NULL. + * This way the method will not wait for response and will return + * immediately after sending the command. * * @param net_idx Network index to encrypt with. * @param addr Target node address. - * @param val New Private GATT Proxy value. Returns response status on - * success. + * @param val New Private GATT Proxy value. + * @param rsp If set, returns response status on success. * * @return 0 on success, or (negative) error code otherwise. */ int bt_mesh_priv_beacon_cli_gatt_proxy_set(uint16_t net_idx, uint16_t addr, - uint8_t *val); + uint8_t val, uint8_t *rsp); /** @brief Get the target's Private GATT Proxy state. * @@ -145,16 +155,21 @@ int bt_mesh_priv_beacon_cli_gatt_proxy_get(uint16_t net_idx, uint16_t addr, uint8_t *val); /** @brief Set the target's Private Node Identity state. + * + * This method can be used asynchronously by setting @p rsp as NULL. + * This way the method will not wait for response and will return + * immediately after sending the command. * * @param net_idx Network index to encrypt with. * @param addr Target node address. - * @param val New Private Node Identity value. Returns response status on - * success. + * @param val New Private Node Identity value. + * @param rsp If set, returns response status on success. * * @return 0 on success, or (negative) error code otherwise. */ int bt_mesh_priv_beacon_cli_node_id_set(uint16_t net_idx, uint16_t addr, - struct bt_mesh_priv_node_id *val); + struct bt_mesh_priv_node_id *val, + struct bt_mesh_priv_node_id *rsp); /** @brief Get the target's Private Node Identity state. * diff --git a/subsys/bluetooth/mesh/priv_beacon_cli.c b/subsys/bluetooth/mesh/priv_beacon_cli.c index 0611aeb7705..7d679f0e322 100644 --- a/subsys/bluetooth/mesh/priv_beacon_cli.c +++ b/subsys/bluetooth/mesh/priv_beacon_cli.c @@ -156,13 +156,14 @@ const struct bt_mesh_model_cb bt_mesh_priv_beacon_cli_cb = { .init = priv_beacon_cli_init, }; -int bt_mesh_priv_beacon_cli_set(uint16_t net_idx, uint16_t addr, struct bt_mesh_priv_beacon *val) +int bt_mesh_priv_beacon_cli_set(uint16_t net_idx, uint16_t addr, struct bt_mesh_priv_beacon *val, + struct bt_mesh_priv_beacon *rsp) { struct bt_mesh_msg_ctx ctx = BT_MESH_MSG_CTX_INIT_DEV(net_idx, addr); const struct bt_mesh_msg_rsp_ctx rsp_ctx = { .ack = &cli->ack_ctx, .op = OP_PRIV_BEACON_STATUS, - .user_data = val, + .user_data = rsp, .timeout = msg_timeout, }; @@ -174,7 +175,7 @@ int bt_mesh_priv_beacon_cli_set(uint16_t net_idx, uint16_t addr, struct bt_mesh_ net_buf_simple_add_u8(&buf, val->rand_interval); } - return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, val ? &rsp_ctx : NULL); + return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, rsp ? &rsp_ctx : NULL); } int bt_mesh_priv_beacon_cli_get(uint16_t net_idx, uint16_t addr, struct bt_mesh_priv_beacon *val) @@ -193,27 +194,28 @@ int bt_mesh_priv_beacon_cli_get(uint16_t net_idx, uint16_t addr, struct bt_mesh_ return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, val ? &rsp_ctx : NULL); } -int bt_mesh_priv_beacon_cli_gatt_proxy_set(uint16_t net_idx, uint16_t addr, uint8_t *val) +int bt_mesh_priv_beacon_cli_gatt_proxy_set(uint16_t net_idx, uint16_t addr, uint8_t val, + uint8_t *rsp) { struct bt_mesh_msg_ctx ctx = BT_MESH_MSG_CTX_INIT_DEV(net_idx, addr); const struct bt_mesh_msg_rsp_ctx rsp_ctx = { .ack = &cli->ack_ctx, .op = OP_PRIV_GATT_PROXY_STATUS, - .user_data = val, + .user_data = rsp, .timeout = msg_timeout, }; - if (!val || (*val != BT_MESH_GATT_PROXY_DISABLED && - *val != BT_MESH_GATT_PROXY_ENABLED)) { + if ((val != BT_MESH_GATT_PROXY_DISABLED && + val != BT_MESH_GATT_PROXY_ENABLED)) { return -EINVAL; } BT_MESH_MODEL_BUF_DEFINE(buf, OP_PRIV_GATT_PROXY_SET, 1); bt_mesh_model_msg_init(&buf, OP_PRIV_GATT_PROXY_SET); - net_buf_simple_add_u8(&buf, *val); + net_buf_simple_add_u8(&buf, val); - return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, val ? &rsp_ctx : NULL); + return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, rsp ? &rsp_ctx : NULL); } int bt_mesh_priv_beacon_cli_gatt_proxy_get(uint16_t net_idx, uint16_t addr, uint8_t *val) @@ -233,17 +235,18 @@ int bt_mesh_priv_beacon_cli_gatt_proxy_get(uint16_t net_idx, uint16_t addr, uint } int bt_mesh_priv_beacon_cli_node_id_set(uint16_t net_idx, uint16_t addr, - struct bt_mesh_priv_node_id *val) + struct bt_mesh_priv_node_id *val, + struct bt_mesh_priv_node_id *rsp) { struct bt_mesh_msg_ctx ctx = BT_MESH_MSG_CTX_INIT_DEV(net_idx, addr); const struct bt_mesh_msg_rsp_ctx rsp_ctx = { .ack = &cli->ack_ctx, .op = OP_PRIV_NODE_ID_STATUS, - .user_data = val, + .user_data = rsp, .timeout = msg_timeout, }; - if (!val || val->net_idx > 0xfff || + if (val->net_idx > 0xfff || (val->state != BT_MESH_NODE_IDENTITY_STOPPED && val->state != BT_MESH_NODE_IDENTITY_RUNNING)) { return -EINVAL; @@ -255,7 +258,7 @@ int bt_mesh_priv_beacon_cli_node_id_set(uint16_t net_idx, uint16_t addr, net_buf_simple_add_le16(&buf, val->net_idx); net_buf_simple_add_u8(&buf, val->state); - return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, val ? &rsp_ctx : NULL); + return bt_mesh_msg_ackd_send(cli->model, &ctx, &buf, rsp ? &rsp_ctx : NULL); } int bt_mesh_priv_beacon_cli_node_id_get(uint16_t net_idx, uint16_t addr, uint16_t key_net_idx, diff --git a/subsys/bluetooth/mesh/shell/priv_beacon.c b/subsys/bluetooth/mesh/shell/priv_beacon.c index 296d96b114e..71b0da91e50 100644 --- a/subsys/bluetooth/mesh/shell/priv_beacon.c +++ b/subsys/bluetooth/mesh/shell/priv_beacon.c @@ -48,7 +48,7 @@ static int cmd_priv_beacon_set(const struct shell *sh, size_t argc, char *argv[] err = bt_mesh_priv_beacon_cli_set(bt_mesh_shell_target_ctx.net_idx, bt_mesh_shell_target_ctx.dst, - &val); + &val, &val); if (err) { shell_error(sh, "Failed to send Private Beacon Set (err %d)", err); return 0; @@ -86,7 +86,7 @@ static int cmd_priv_gatt_proxy_set(const struct shell *sh, size_t argc, char *ar } err = bt_mesh_priv_beacon_cli_gatt_proxy_set(bt_mesh_shell_target_ctx.net_idx, - bt_mesh_shell_target_ctx.dst, &state); + bt_mesh_shell_target_ctx.dst, state, &state); if (err) { shell_error(sh, "Failed to send Private GATT Proxy Set (err %d)", err); return 0; @@ -130,7 +130,7 @@ static int cmd_priv_node_id_set(const struct shell *sh, size_t argc, char *argv[ } err = bt_mesh_priv_beacon_cli_node_id_set(bt_mesh_shell_target_ctx.net_idx, - bt_mesh_shell_target_ctx.dst, &val); + bt_mesh_shell_target_ctx.dst, &val, &val); if (err) { shell_error(sh, "Failed to send Private Node Identity Set (err %d)", err); return 0; diff --git a/tests/bluetooth/tester/src/btp_mesh.c b/tests/bluetooth/tester/src/btp_mesh.c index 96ce410eed8..ab5fcf515bd 100644 --- a/tests/bluetooth/tester/src/btp_mesh.c +++ b/tests/bluetooth/tester/src/btp_mesh.c @@ -820,7 +820,7 @@ static uint8_t priv_beacon_set(const void *cmd, uint16_t cmd_len, val.enabled = cp->enabled; val.rand_interval = cp->rand_interval; - err = bt_mesh_priv_beacon_cli_set(net.net_idx, cp->dst, &val); + err = bt_mesh_priv_beacon_cli_set(net.net_idx, cp->dst, &val, &val); if (err) { LOG_ERR("Failed to send Private Beacon Set (err %d)", err); return BTP_STATUS_FAILED; @@ -857,7 +857,7 @@ static uint8_t priv_gatt_proxy_set(const void *cmd, uint16_t cmd_len, state = cp->state; - err = bt_mesh_priv_beacon_cli_gatt_proxy_set(net.net_idx, cp->dst, &state); + err = bt_mesh_priv_beacon_cli_gatt_proxy_set(net.net_idx, cp->dst, state, &state); if (err) { LOG_ERR("Failed to send Private GATT Proxy Set (err %d)", err); return BTP_STATUS_FAILED; @@ -896,7 +896,7 @@ static uint8_t priv_node_id_set(const void *cmd, uint16_t cmd_len, val.net_idx = cp->net_idx; val.state = cp->state; - err = bt_mesh_priv_beacon_cli_node_id_set(net.net_idx, cp->dst, &val); + err = bt_mesh_priv_beacon_cli_node_id_set(net.net_idx, cp->dst, &val, &val); if (err) { LOG_ERR("Failed to send Private Node Identity Set (err %d)", err); return BTP_STATUS_FAILED; diff --git a/tests/bsim/bluetooth/mesh/src/test_beacon.c b/tests/bsim/bluetooth/mesh/src/test_beacon.c index ef1ce8b9f74..e93c2301e46 100644 --- a/tests/bsim/bluetooth/mesh/src/test_beacon.c +++ b/tests/bsim/bluetooth/mesh/src/test_beacon.c @@ -1131,7 +1131,7 @@ static void tx_priv_setup(void) FAIL("Beacon set failed (err %d, status %u)", err, status); } - err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } @@ -1169,7 +1169,7 @@ static void test_tx_priv_adv(void) val.enabled = 1; val.rand_interval = 1; - err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } @@ -1178,7 +1178,7 @@ static void test_tx_priv_adv(void) val.rand_interval = 0; - err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } @@ -1187,7 +1187,7 @@ static void test_tx_priv_adv(void) val.rand_interval = 0; - err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } @@ -1196,7 +1196,7 @@ static void test_tx_priv_adv(void) val.rand_interval = 3; - err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, tx_cfg.addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } @@ -1328,7 +1328,7 @@ static void test_rx_priv_invalid(void) FAIL("Beacon set failed (err %d, status %u)", err, status); } - err = bt_mesh_priv_beacon_cli_set(0, rx_cfg.addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, rx_cfg.addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } @@ -1361,7 +1361,7 @@ static void toggle_priv_beacon(uint16_t addr, uint8_t enabled) val.enabled = enabled; val.rand_interval = 1; - err = bt_mesh_priv_beacon_cli_set(0, addr, &val); + err = bt_mesh_priv_beacon_cli_set(0, addr, &val, &val); if (err) { FAIL("Failed to enable Private Beacon (err=%d)", err); } diff --git a/tests/bsim/bluetooth/mesh/src/test_persistence.c b/tests/bsim/bluetooth/mesh/src/test_persistence.c index df3eaa19989..4bf1f9faa93 100644 --- a/tests/bsim/bluetooth/mesh/src/test_persistence.c +++ b/tests/bsim/bluetooth/mesh/src/test_persistence.c @@ -953,14 +953,16 @@ static void test_cfg_save(void) .rand_interval = current_stack_cfg->priv_beacon_int, }; - err = bt_mesh_priv_beacon_cli_set(test_netkey_idx, TEST_ADDR, &priv_beacon_state); + err = bt_mesh_priv_beacon_cli_set(test_netkey_idx, TEST_ADDR, &priv_beacon_state, + &priv_beacon_state); if (err) { FAIL("Failed to enable Private Beacon (err %d)", err); } uint8_t priv_beacon_gatt = current_stack_cfg->priv_beacon_gatt; - err = bt_mesh_priv_beacon_cli_gatt_proxy_set(test_netkey_idx, TEST_ADDR, &priv_beacon_gatt); + err = bt_mesh_priv_beacon_cli_gatt_proxy_set(test_netkey_idx, TEST_ADDR, priv_beacon_gatt, + &priv_beacon_gatt); if (err) { FAIL("Failed to enable Private Beacon GATT proxy (err %d)", err); }