From 3f5aaf11dbc067a9d79a96723e353620827938ff Mon Sep 17 00:00:00 2001 From: xiongweichao Date: Tue, 26 Apr 2022 17:24:51 +0800 Subject: [PATCH 1/4] Added esp_spp_vfs_unregister() to free memory allocated by esp_spp_vfs_register() --- .../bt/host/bluedroid/api/esp_spp_api.c | 21 ++++- .../bluedroid/api/include/api/esp_spp_api.h | 29 ++++++ .../btc/profile/std/include/btc_spp.h | 3 +- .../bluedroid/btc/profile/std/spp/btc_spp.c | 93 +++++++++++++++---- 4 files changed, 124 insertions(+), 22 deletions(-) diff --git a/components/bt/host/bluedroid/api/esp_spp_api.c b/components/bt/host/bluedroid/api/esp_spp_api.c index 962a20a177..0578691a22 100644 --- a/components/bt/host/bluedroid/api/esp_spp_api.c +++ b/components/bt/host/bluedroid/api/esp_spp_api.c @@ -217,9 +217,28 @@ esp_err_t esp_spp_write(uint32_t handle, int len, uint8_t *p_data) esp_err_t esp_spp_vfs_register(void) { + btc_msg_t msg; + btc_spp_args_t arg; ESP_BLUEDROID_STATUS_CHECK(ESP_BLUEDROID_STATUS_ENABLED); - return btc_spp_vfs_register(); + msg.sig = BTC_SIG_API_CALL; + msg.pid = BTC_PID_SPP; + msg.act = BTC_SPP_ACT_VFS_REGISTER; + + return (btc_transfer_context(&msg, &arg, sizeof(btc_spp_args_t), NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); +} + +esp_err_t esp_spp_vfs_unregister(void) +{ + btc_msg_t msg; + btc_spp_args_t arg; + ESP_BLUEDROID_STATUS_CHECK(ESP_BLUEDROID_STATUS_ENABLED); + + msg.sig = BTC_SIG_API_CALL; + msg.pid = BTC_PID_SPP; + msg.act = BTC_SPP_ACT_VFS_UNREGISTER; + + return (btc_transfer_context(&msg, &arg, sizeof(btc_spp_args_t), NULL, NULL) == BT_STATUS_SUCCESS ? ESP_OK : ESP_FAIL); } #endif ///defined BTC_SPP_INCLUDED && BTC_SPP_INCLUDED == TRUE diff --git a/components/bt/host/bluedroid/api/include/api/esp_spp_api.h b/components/bt/host/bluedroid/api/include/api/esp_spp_api.h index a874c25c89..60008a646d 100644 --- a/components/bt/host/bluedroid/api/include/api/esp_spp_api.h +++ b/components/bt/host/bluedroid/api/include/api/esp_spp_api.h @@ -90,6 +90,8 @@ typedef enum { ESP_SPP_WRITE_EVT = 33, /*!< When SPP write operation completes, the event comes, only for ESP_SPP_MODE_CB */ ESP_SPP_SRV_OPEN_EVT = 34, /*!< When SPP Server connection open, the event comes */ ESP_SPP_SRV_STOP_EVT = 35, /*!< When SPP server stopped, the event comes */ + ESP_SPP_VFS_REGISTER_EVT = 36, /*!< When SPP VFS register, the event comes */ + ESP_SPP_VFS_UNREGISTER_EVT = 37, /*!< When SPP VFS unregister, the event comes */ } esp_spp_cb_event_t; @@ -208,6 +210,20 @@ typedef union { uint32_t handle; /*!< The connection handle */ bool cong; /*!< TRUE, congested. FALSE, uncongested */ } cong; /*!< SPP callback param of ESP_SPP_CONG_EVT */ + + /** + * @brief ESP_SPP_VFS_REGISTER_EVT + */ + struct spp_vfs_register_evt_param { + esp_spp_status_t status; /*!< status */ + } vfs_register; /*!< SPP callback param of ESP_SPP_VFS_REGISTER_EVT */ + + /** + * @brief ESP_SPP_VFS_UNREGISTER_EVT + */ + struct spp_vfs_unregister_evt_param { + esp_spp_status_t status; /*!< status */ + } vfs_unregister; /*!< SPP callback param of ESP_SPP_VFS_UNREGISTER_EVT */ } esp_spp_cb_param_t; /*!< SPP callback parameter union type */ /** @@ -388,6 +404,8 @@ esp_err_t esp_spp_write(uint32_t handle, int len, uint8_t *p_data); /** * @brief This function is used to register VFS. * For now, SPP only supports write, read and close. + * When the operation is completed, the callback function will be called with ESP_SPP_VFS_REGISTER_EVT. + * This function must be called after esp_spp_init()/esp_spp_enhanced_init() successful and before esp_spp_deinit(). * * @return * - ESP_OK: success @@ -395,6 +413,17 @@ esp_err_t esp_spp_write(uint32_t handle, int len, uint8_t *p_data); */ esp_err_t esp_spp_vfs_register(void); +/** + * @brief This function is used to unregister VFS. + * When the operation is completed, the callback function will be called with ESP_SPP_VFS_UNREGISTER_EVT. + * This function must be called after esp_spp_vfs_register() successful and before esp_spp_deinit(). + * + * @return + * - ESP_OK: success + * - other: failed + */ +esp_err_t esp_spp_vfs_unregister(void); + #ifdef __cplusplus } #endif diff --git a/components/bt/host/bluedroid/btc/profile/std/include/btc_spp.h b/components/bt/host/bluedroid/btc/profile/std/include/btc_spp.h index 4fe0b06607..b34981323b 100644 --- a/components/bt/host/bluedroid/btc/profile/std/include/btc_spp.h +++ b/components/bt/host/bluedroid/btc/profile/std/include/btc_spp.h @@ -29,6 +29,8 @@ typedef enum { BTC_SPP_ACT_START_SRV, BTC_SPP_ACT_STOP_SRV, BTC_SPP_ACT_WRITE, + BTC_SPP_ACT_VFS_REGISTER, + BTC_SPP_ACT_VFS_UNREGISTER, } btc_spp_act_t; /* btc_spp_args_t */ @@ -88,6 +90,5 @@ void btc_spp_arg_deep_copy(btc_msg_t *msg, void *p_dest, void *p_src); void btc_spp_arg_deep_free(btc_msg_t *msg); esp_err_t spp_send_data_to_btc(uint32_t handle, int len, uint8_t *p_data, esp_spp_mode_t spp_mode); -esp_err_t btc_spp_vfs_register(void); #endif ///defined BTC_SPP_INCLUDED && BTC_SPP_INCLUDED == TRUE #endif ///__BTC_SPP_H__ diff --git a/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c b/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c index dc695a94ae..212edca60d 100644 --- a/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c +++ b/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c @@ -92,6 +92,9 @@ static spp_local_param_t *spp_local_param_ptr; #define spp_local_param (*spp_local_param_ptr) #endif +static void btc_spp_vfs_register(void); +static void btc_spp_vfs_unregister(void); + static void spp_osi_free(void *p) { osi_free(p); @@ -546,6 +549,9 @@ static void btc_spp_init(btc_spp_args_t *arg) ret = ESP_SPP_NO_RESOURCE; break; } + if (arg->init.mode == ESP_SPP_MODE_VFS) { + spp_local_param.spp_vfs_id = -1; + } spp_local_param.spp_mode = arg->init.mode; spp_local_param.spp_slot_id = 0; spp_local_param.tx_buffer_size = arg->init.tx_buffer_size; @@ -988,6 +994,12 @@ void btc_spp_call_handler(btc_msg_t *msg) case BTC_SPP_ACT_WRITE: btc_spp_write(arg); break; + case BTC_SPP_ACT_VFS_REGISTER: + btc_spp_vfs_register(); + break; + case BTC_SPP_ACT_VFS_UNREGISTER: + btc_spp_vfs_unregister(); + break; default: BTC_TRACE_ERROR("%s: Unhandled event (%d)!\n", __FUNCTION__, msg->act); break; @@ -1305,6 +1317,12 @@ void btc_spp_cb_handler(btc_msg_t *msg) vEventGroupDelete(spp_local_param.tx_event_group); spp_local_param.tx_event_group = NULL; } + if (spp_local_param.spp_mode == ESP_SPP_MODE_VFS) { + if (spp_local_param.spp_vfs_id != -1) { + esp_vfs_unregister_with_id(spp_local_param.spp_vfs_id); + spp_local_param.spp_vfs_id = -1; + } + } #if SPP_DYNAMIC_MEMORY == TRUE osi_free(spp_local_param_ptr); spp_local_param_ptr = NULL; @@ -1599,30 +1617,65 @@ static ssize_t spp_vfs_read(int fd, void * dst, size_t size) return item_size; } -esp_err_t btc_spp_vfs_register(void) +static void btc_spp_vfs_register(void) { - if (!is_spp_init()) { - BTC_TRACE_ERROR("%s SPP have not been init\n", __func__); - return ESP_FAIL; - } + esp_spp_status_t ret = ESP_SPP_SUCCESS; + esp_spp_cb_param_t param; - esp_vfs_t vfs = { - .flags = ESP_VFS_FLAG_DEFAULT, - .write = spp_vfs_write, - .open = NULL, - .fstat = NULL, - .close = spp_vfs_close, - .read = spp_vfs_read, - .fcntl = NULL - }; + do { + if (!is_spp_init()) { + BTC_TRACE_ERROR("%s SPP have not been init\n", __func__); + ret = ESP_SPP_NEED_INIT; + break; + } - // No FD range is registered here: spp_vfs_id is used to register/unregister - // file descriptors - if (esp_vfs_register_with_id(&vfs, NULL, &spp_local_param.spp_vfs_id) != ESP_OK) { - return ESP_FAIL; - } + esp_vfs_t vfs = { + .flags = ESP_VFS_FLAG_DEFAULT, + .write = spp_vfs_write, + .open = NULL, + .fstat = NULL, + .close = spp_vfs_close, + .read = spp_vfs_read, + .fcntl = NULL + }; - return ESP_OK; + // No FD range is registered here: spp_vfs_id is used to register/unregister + // file descriptors + if (esp_vfs_register_with_id(&vfs, NULL, &spp_local_param.spp_vfs_id) != ESP_OK) { + ret = ESP_SPP_FAILURE; + break; + } + } while (0); + + param.vfs_register.status = ret; + btc_spp_cb_to_app(ESP_SPP_VFS_REGISTER_EVT, ¶m); +} + +static void btc_spp_vfs_unregister(void) +{ + esp_spp_status_t ret = ESP_SPP_SUCCESS; + esp_spp_cb_param_t param; + + do { + if (!is_spp_init()) { + BTC_TRACE_ERROR("%s SPP have not been init\n", __func__); + ret = ESP_SPP_NEED_INIT; + break; + } + + if (spp_local_param.spp_mode == ESP_SPP_MODE_VFS) { + if (spp_local_param.spp_vfs_id != -1) { + if (esp_vfs_unregister_with_id(spp_local_param.spp_vfs_id) != ESP_OK) { + ret = ESP_SPP_FAILURE; + break; + } + spp_local_param.spp_vfs_id = -1; + } + } + } while (0); + + param.vfs_unregister.status = ret; + btc_spp_cb_to_app(ESP_SPP_VFS_UNREGISTER_EVT, ¶m); } #endif ///defined BTC_SPP_INCLUDED && BTC_SPP_INCLUDED == TRUE From 29b718bdf5da7faa1c35fa340df86306376fb77e Mon Sep 17 00:00:00 2001 From: xiongweichao Date: Sat, 7 May 2022 20:07:23 +0800 Subject: [PATCH 2/4] Fixed memory leak when SPP initialization failed --- .../bt/host/bluedroid/btc/profile/std/spp/btc_spp.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c b/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c index 212edca60d..6c6eb8aa9d 100644 --- a/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c +++ b/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c @@ -540,12 +540,20 @@ static void btc_spp_init(btc_spp_args_t *arg) if (osi_mutex_new(&spp_local_param.spp_slot_mutex) != 0) { BTC_TRACE_ERROR("%s osi_mutex_new failed\n", __func__); +#if SPP_DYNAMIC_MEMORY == TRUE + osi_free(spp_local_param_ptr); + spp_local_param_ptr = NULL; +#endif ret = ESP_SPP_NO_RESOURCE; break; } if ((spp_local_param.tx_event_group = xEventGroupCreate()) == NULL) { BTC_TRACE_ERROR("%s create tx_event_group failed\n", __func__); osi_mutex_free(&spp_local_param.spp_slot_mutex); +#if SPP_DYNAMIC_MEMORY == TRUE + osi_free(spp_local_param_ptr); + spp_local_param_ptr = NULL; +#endif ret = ESP_SPP_NO_RESOURCE; break; } From 474cf2cf64754ba82afcbe8dabf6cba70dbd9d5b Mon Sep 17 00:00:00 2001 From: xiongweichao Date: Wed, 6 Jul 2022 10:46:41 +0800 Subject: [PATCH 3/4] Modified spp vfs example --- .../classic_bt/bt_spp_vfs_acceptor/main/main.c | 9 ++++++++- .../classic_bt/bt_spp_vfs_initiator/main/main.c | 13 ++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_acceptor/main/main.c b/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_acceptor/main/main.c index abcdde4d8b..7621568b4e 100644 --- a/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_acceptor/main/main.c +++ b/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_acceptor/main/main.c @@ -102,7 +102,6 @@ static void esp_spp_cb(uint16_t e, void *p) ESP_LOGI(SPP_TAG, "ESP_SPP_INIT_EVT"); /* Enable SPP VFS mode */ esp_spp_vfs_register(); - esp_spp_start_srv(sec_mask, role_slave, 0, SPP_SERVER_NAME); } else { ESP_LOGE(SPP_TAG, "ESP_SPP_INIT_EVT status:%d", param->init.status); } @@ -137,6 +136,14 @@ static void esp_spp_cb(uint16_t e, void *p) spp_wr_task_start_up(spp_read_handle, param->srv_open.fd); } break; + case ESP_SPP_VFS_REGISTER_EVT: + if (param->vfs_register.status == ESP_SPP_SUCCESS) { + ESP_LOGI(SPP_TAG, "ESP_SPP_VFS_REGISTER_EVT"); + esp_spp_start_srv(sec_mask, role_slave, 0, SPP_SERVER_NAME); + } else { + ESP_LOGE(SPP_TAG, "ESP_SPP_VFS_REGISTER_EVT status:%d", param->vfs_register.status); + } + break; default: break; } diff --git a/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_initiator/main/main.c b/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_initiator/main/main.c index dea4c6d2bf..fc2e62c300 100644 --- a/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_initiator/main/main.c +++ b/examples/bluetooth/bluedroid/classic_bt/bt_spp_vfs_initiator/main/main.c @@ -147,9 +147,6 @@ static void esp_spp_cb(uint16_t e, void *p) ESP_LOGI(SPP_TAG, "ESP_SPP_INIT_EVT"); /* Enable SPP VFS mode */ esp_spp_vfs_register(); - esp_bt_dev_set_device_name(EXAMPLE_DEVICE_NAME); - esp_bt_gap_set_scan_mode(ESP_BT_CONNECTABLE, ESP_BT_GENERAL_DISCOVERABLE); - esp_bt_gap_start_discovery(inq_mode, inq_len, inq_num_rsps); } else { ESP_LOGE(SPP_TAG, "ESP_SPP_INIT_EVT status:%d", param->init.status); } @@ -193,6 +190,16 @@ static void esp_spp_cb(uint16_t e, void *p) case ESP_SPP_SRV_OPEN_EVT: ESP_LOGI(SPP_TAG, "ESP_SPP_SRV_OPEN_EVT"); break; + case ESP_SPP_VFS_REGISTER_EVT: + if (param->vfs_register.status == ESP_SPP_SUCCESS) { + ESP_LOGI(SPP_TAG, "ESP_SPP_VFS_REGISTER_EVT"); + esp_bt_dev_set_device_name(EXAMPLE_DEVICE_NAME); + esp_bt_gap_set_scan_mode(ESP_BT_CONNECTABLE, ESP_BT_GENERAL_DISCOVERABLE); + esp_bt_gap_start_discovery(inq_mode, inq_len, inq_num_rsps); + } else { + ESP_LOGE(SPP_TAG, "ESP_SPP_VFS_REGISTER_EVT status:%d", param->vfs_register.status); + } + break; default: break; } From 32a50118a4dfd37837d28bb5942e0f48824d188d Mon Sep 17 00:00:00 2001 From: xiongweichao Date: Wed, 6 Jul 2022 11:13:25 +0800 Subject: [PATCH 4/4] Assert when malloc user_data fail --- .../bt/host/bluedroid/btc/profile/std/spp/btc_spp.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c b/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c index 6c6eb8aa9d..41fad3d780 100644 --- a/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c +++ b/components/bt/host/bluedroid/btc/profile/std/spp/btc_spp.c @@ -499,6 +499,7 @@ static void btc_spp_dm_inter_cb(tBTA_JV_EVT event, tBTA_JV *p_data, void *user_d user_data->slot_id = slot->id; } else { BTC_TRACE_ERROR("%s unable to malloc user data!", __func__); + assert(0); } BTA_JvFreeChannel(slot->scn, BTA_JV_CONN_TYPE_RFCOMM, (tBTA_JV_RFCOMM_CBACK *)btc_spp_rfcomm_inter_cb, (void *)user_data); @@ -608,11 +609,8 @@ static void btc_spp_uninit(void) user_data->server_status = BTA_JV_SERVER_RUNNING; user_data->slot_id = spp_local_param.spp_slots[i]->id; } else { - esp_spp_cb_param_t param; BTC_TRACE_ERROR("%s unable to malloc user data!", __func__); - param.srv_stop.status = ESP_SPP_NO_RESOURCE; - param.srv_stop.scn = spp_local_param.spp_slots[i]->scn; - btc_spp_cb_to_app(ESP_SPP_SRV_STOP_EVT, ¶m); + assert(0); } BTA_JvFreeChannel(spp_local_param.spp_slots[i]->scn, BTA_JV_CONN_TYPE_RFCOMM, (tBTA_JV_RFCOMM_CBACK *)btc_spp_rfcomm_inter_cb, (void *)user_data); @@ -789,7 +787,7 @@ static void btc_spp_stop_srv(btc_spp_args_t *arg) } osi_mutex_lock(&spp_local_param.spp_slot_mutex, OSI_MUTEX_MAX_TIMEOUT); - // [1] find all server + // [1] find all unconnected server for (i = 1; i <= MAX_RFC_PORTS; i++) { if (spp_local_param.spp_slots[i] != NULL && !spp_local_param.spp_slots[i]->connected && spp_local_param.spp_slots[i]->sdp_handle > 0) { @@ -844,11 +842,8 @@ static void btc_spp_stop_srv(btc_spp_args_t *arg) user_data->server_status = BTA_JV_SERVER_RUNNING; user_data->slot_id = spp_local_param.spp_slots[i]->id; } else { - esp_spp_cb_param_t param; BTC_TRACE_ERROR("%s unable to malloc user data!", __func__); - param.srv_stop.status = ESP_SPP_NO_RESOURCE; - param.srv_stop.scn = spp_local_param.spp_slots[i]->scn; - btc_spp_cb_to_app(ESP_SPP_SRV_STOP_EVT, ¶m); + assert(0); } BTA_JvFreeChannel(spp_local_param.spp_slots[i]->scn, BTA_JV_CONN_TYPE_RFCOMM, (tBTA_JV_RFCOMM_CBACK *)btc_spp_rfcomm_inter_cb, (void *)user_data);