Avoid malloc/free while in HCI callbacks (#2219)

Bluetooth operates at IRQ level, so using std::list (which needs to
new and delete objects) is not legal.  Use a fixed, preallocated
vector instead.

Author: earlephilhower

docs/hidmaster.rst

@@ -11,7 +11,7 @@ BTDeviceInfo Class
 ------------------
 
 The ``BluetoothHCI`` class implements a scanning function for classic
-and BLE devices and can return a ``std::list`` of discovered devices to an application.
+and BLE devices and can return a ``std::vector`` of discovered devices to an application.
 Iterate over the list using any of the STL iteration methods (i.e. ``for (auto e : list)``).
 The elements of this list are ``BTDeviceInfo`` objects which have the following
 member functions:
@@ -179,7 +179,7 @@ bool BluetoothHIDMaster::hciRunning()
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Returns if the Bluetooth stack has passed the initial HCI start up phase.  Until this returns ``true`` no Bluetooth operations can be performed.
 
-std::list<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async)
+std::vector<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Passes through the ``BluetoothHCI::scan()`` function to manually scan for a list of nearby devices.  If you want to connect to the first found device, this is not needed.
 

libraries/BluetoothAudio/src/A2DPSource.h

@@ -104,7 +104,7 @@ class A2DPSource : public Stream {
 
     bool begin();
 
-    std::list<BTDeviceInfo> scan(uint32_t mask = BluetoothHCI::speaker_cod, int scanTimeSec = 5, bool async = false) {
+    std::vector<BTDeviceInfo> scan(uint32_t mask = BluetoothHCI::speaker_cod, int scanTimeSec = 5, bool async = false) {
         return _hci.scan(mask, scanTimeSec, async);
     }
 

libraries/BluetoothHCI/src/BluetoothDevice.h

@@ -24,59 +24,60 @@
 
 class BTDeviceInfo {
 public:
+    // Classic Bluetooth Device
     BTDeviceInfo(uint32_t dc, const uint8_t addr[6], int rssi, const char *name) {
         _deviceClass = dc;
         memcpy(_address, addr, sizeof(_address));
         _addressType = -1;
         sprintf(_addressString, "%02x:%02x:%02x:%02x:%02x:%02x", addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
         _rssi = rssi;
-        _name = strdup(name);
+        strncpy(_name, name, sizeof(_name));
+        _name[sizeof(_name) - 1] = 0;
     }
+
+    // Bluetooth BLE Device
     BTDeviceInfo(uint32_t dc, const uint8_t addr[6], int addressType, int rssi, const char *name, size_t nameLen) {
         _deviceClass = dc;
         memcpy(_address, addr, sizeof(_address));
         sprintf(_addressString, "%02x:%02x:%02x:%02x:%02x:%02x", addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
         _addressType = addressType;
         _rssi = rssi;
-        _name = (char *)malloc(nameLen + 1);
-        memcpy(_name, name, nameLen);
-        _name[nameLen] = 0;
-    }
-    // Copy constructor to ensure we deep-copy the string
-    BTDeviceInfo(const BTDeviceInfo &b) {
-        _deviceClass = b._deviceClass;
-        memcpy(_address, b._address, sizeof(_address));
-        _addressType = b._addressType;
-        memcpy(_addressString, b._addressString, sizeof(_addressString));
-        _rssi = b._rssi;
-        _name = strdup(b._name);
+        memcpy(_name, name, std::min(nameLen, sizeof(_name)));
+        _name[std::min(nameLen, sizeof(_name) - 1)] = 0;
     }
+
     ~BTDeviceInfo() {
-        free(_name);
     }
+
     uint32_t deviceClass() {
         return _deviceClass;
     }
+
     const uint8_t *address() {
         return _address;
     }
+
     const char *addressString() {
         return _addressString;
     }
+
     int rssi() {
         return _rssi;
     }
+
     const char *name() {
         return _name;
     }
+
     int addressType() {
         return _addressType;
     }
+
 private:
     uint32_t _deviceClass;
     uint8_t _address[6];
     int _addressType;
     char _addressString[18];
     int8_t _rssi;
-    char *_name;
+    char _name[241];
 };

libraries/BluetoothHCI/src/BluetoothHCI.cpp

@@ -103,8 +103,9 @@ bool BluetoothHCI::running() {
     return _hciRunning;
 }
 
-std::list<BTDeviceInfo> BluetoothHCI::scan(uint32_t mask, int scanTimeSec, bool async) {
+std::vector<BTDeviceInfo> BluetoothHCI::scan(uint32_t mask, int scanTimeSec, bool async) {
     _scanMask = mask;
+    _btdList.reserve(MAX_DEVICES_TO_DISCOVER);
     _btdList.clear();
     if (!_running) {
         return _btdList;
@@ -135,8 +136,9 @@ std::list<BTDeviceInfo> BluetoothHCI::scan(uint32_t mask, int scanTimeSec, bool
     return _btdList;
 }
 
-std::list<BTDeviceInfo> BluetoothHCI::scanBLE(uint32_t uuid, int scanTimeSec) {
+std::vector<BTDeviceInfo> BluetoothHCI::scanBLE(uint32_t uuid, int scanTimeSec) {
     _scanMask = uuid;
+    _btdList.reserve(MAX_DEVICES_TO_DISCOVER);
     _btdList.clear();
     if (!_running) {
         return _btdList;
@@ -168,6 +170,10 @@ std::list<BTDeviceInfo> BluetoothHCI::scanBLE(uint32_t uuid, int scanTimeSec) {
     return _btdList;
 }
 
+void BluetoothHCI::scanFree() {
+    _btdList.clear();
+    _btdList.shrink_to_fit();
+}
 
 void BluetoothHCI::parse_advertisement_data(uint8_t *packet) {
     bd_addr_t address;
@@ -298,7 +304,9 @@ void BluetoothHCI::parse_advertisement_data(uint8_t *packet) {
         }
         if (!seen) {
             BTDeviceInfo btd(uuid, address, address_type, rssi, nameptr ? nameptr : "", nameptr ? namelen : 0);
-            _btdList.push_back(btd);
+            if (_btdList.size() < MAX_DEVICES_TO_DISCOVER) {
+                _btdList.push_back(btd);
+            }
         }
     }
 }
@@ -394,7 +402,9 @@ void BluetoothHCI::hci_packet_handler(uint8_t packet_type, uint16_t channel, uin
                 }
             }
             BTDeviceInfo btd(cod, address, rssi, name);
-            _btdList.push_back(btd);
+            if (_btdList.size() < MAX_DEVICES_TO_DISCOVER) {
+                _btdList.push_back(btd);
+            }
         }
         break;
     case GAP_EVENT_INQUIRY_COMPLETE:

libraries/BluetoothHCI/src/BluetoothHCI.h

@@ -39,11 +39,13 @@ class BluetoothHCI {
 
     static const uint32_t speaker_cod = 0x200000 | 0x040000 | 0x000400;  // Service Class: Rendering | Audio, Major Device Class: Audio
     static const uint32_t any_cod = 0;
-    std::list<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
+    std::vector<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
     bool scanAsyncDone();
-    std::list<BTDeviceInfo> scanAsyncResult();
+    std::vector<BTDeviceInfo> scanAsyncResult();
 
-    std::list<BTDeviceInfo> scanBLE(uint32_t uuid, int scanTimeSec = 5);
+    std::vector<BTDeviceInfo> scanBLE(uint32_t uuid, int scanTimeSec = 5);
+
+    void scanFree(); // Free allocated scan buffers
 
     friend class BluetoothHIDMaster;
 
@@ -60,7 +62,11 @@ class BluetoothHCI {
     btstack_packet_callback_registration_t hci_event_callback_registration;
     volatile bool _hciRunning = false;
     uint32_t _scanMask;
-    std::list<BTDeviceInfo> _btdList;
+    // Use a .reserve()'d vector to avoid any memory allocations in the
+    // HCI callback, since the callback happens at IRQ time.  Any more
+    // elements than MAX_DEVICES_TO_DISCOVER will be thrown out
+    enum { MAX_DEVICES_TO_DISCOVER = 32 };
+    std::vector<BTDeviceInfo> _btdList;
     volatile bool _scanning = false;
     bool _running = false;
 

libraries/BluetoothHIDMaster/src/BluetoothHIDMaster.cpp

@@ -166,14 +166,14 @@ void BluetoothHIDMaster::onJoystick(void (*cb)(void *, int, int, int, int, uint8
     _joystickData = cbData;
 }
 
-std::list<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async) {
+std::vector<BTDeviceInfo> BluetoothHIDMaster::scan(uint32_t mask, int scanTimeSec, bool async) {
     return _hci.scan(mask, scanTimeSec, async);
 }
 
 bool BluetoothHIDMaster::scanAsyncDone() {
     return _hci.scanAsyncDone();
 }
-std::list<BTDeviceInfo> BluetoothHIDMaster::scanAsyncResult() {
+std::vector<BTDeviceInfo> BluetoothHIDMaster::scanAsyncResult() {
     return _hci.scanAsyncResult();
 }
 

libraries/BluetoothHIDMaster/src/BluetoothHIDMaster.h

@@ -80,9 +80,9 @@ class BluetoothHIDMaster {
     static const uint32_t mouse_cod = 0x2540;
     static const uint32_t joystick_cod = 0x2508;
     static const uint32_t any_cod = 0;
-    std::list<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
+    std::vector<BTDeviceInfo> scan(uint32_t mask, int scanTimeSec = 5, bool async = false);
     bool scanAsyncDone();
-    std::list<BTDeviceInfo> scanAsyncResult();
+    std::vector<BTDeviceInfo> scanAsyncResult();
 
     bool connect(const uint8_t *addr);
     bool connectKeyboard();