Merge branch 'bugfix/spp_vfs_memory_leak' into 'master'

Component_bt/fix esp_spp_vfs_register memory leak

Closes BT-2344

See merge request espressif/esp-idf!17909

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

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

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__

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);
@@ -496,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);
@@ -537,15 +541,26 @@ 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;
         }
+        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;
@@ -594,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, &param);
+                    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);
@@ -775,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) {
@@ -830,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, &param);
+                        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);
@@ -988,6 +997,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 +1320,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;
@@ -1598,30 +1619,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
+        };
+
+        // 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, &param);
+}
+
+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);
 
-    return ESP_OK;
+    param.vfs_unregister.status = ret;
+    btc_spp_cb_to_app(ESP_SPP_VFS_UNREGISTER_EVT, &param);
 }
 
 #endif ///defined BTC_SPP_INCLUDED && BTC_SPP_INCLUDED == TRUE

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;
     }

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;
     }