Fix errors in handling RawAddresses

* In change I8d1bd6914aec55bb53495b1d0d5e3d37b86865e6
  memcmp(a, b, LEN) != 0 should be translated to
  A != B
* memcpy should not be applied to RawAddress objects.
  Assignment operator should be used instead.
* memset should not be applied to RawAddress objects.
  Assignment to RawAddress::kEmpty should be used.
* Fixed a crash in GATT

Bug: 64316340
Test: Unit test, pair with device and transmit
Change-Id: Iceefab821c1d45a88194d87a43a192afa5f263fd
(cherry picked from commit 882aec320e88b0860a3d53db828bb099c7ca2409)

diff --git a/btif/src/btif_av.cc b/btif/src/btif_av.cc
index bc9c282..8074f1f 100644 (file)
--- a/btif/src/btif_av.cc
+++ b/btif/src/btif_av.cc
@@ -333,7 +333,7 @@ static bool btif_av_state_idle_handler(btif_sm_event_t event, void* p_data) {
   switch (event) {
     case BTIF_SM_ENTER_EVT:
       /* clear the peer_bda */
-      memset(&btif_av_cb.peer_bda, 0, sizeof(RawAddress));
+      btif_av_cb.peer_bda = RawAddress::kEmpty;
       btif_av_cb.flags = 0;
       btif_av_cb.edr = 0;
       bta_av_co_init(btif_av_cb.codec_priorities);
@@ -353,12 +353,10 @@ static bool btif_av_state_idle_handler(btif_sm_event_t event, void* p_data) {
     case BTA_AV_PENDING_EVT:
     case BTIF_AV_CONNECT_REQ_EVT: {
       if (event == BTIF_AV_CONNECT_REQ_EVT) {
-        memcpy(&btif_av_cb.peer_bda,
-               ((btif_av_connect_req_t*)p_data)->target_bda,
-               sizeof(RawAddress));
+        btif_av_connect_req_t* connect_req_p = (btif_av_connect_req_t*)p_data;
+        btif_av_cb.peer_bda = *connect_req_p->target_bda;
         BTA_AvOpen(btif_av_cb.peer_bda, btif_av_cb.bta_handle, true,
-                   BTA_SEC_AUTHENTICATE,
-                   ((btif_av_connect_req_t*)p_data)->uuid);
+                   BTA_SEC_AUTHENTICATE, connect_req_p->uuid);
       } else if (event == BTA_AV_PENDING_EVT) {
         btif_av_cb.peer_bda = ((tBTA_AV*)p_data)->pend.bd_addr;
         if (bt_av_src_callbacks != NULL) {
@@ -599,25 +597,23 @@ static bool btif_av_state_opening_handler(btif_sm_event_t event, void* p_data) {
       }
     } break;
 
-    case BTIF_AV_CONNECT_REQ_EVT:
+    case BTIF_AV_CONNECT_REQ_EVT: {
       // Check for device, if same device which moved to opening then ignore
       // callback
-      if (memcmp(((btif_av_connect_req_t*)p_data)->target_bda,
-                 &(btif_av_cb.peer_bda), sizeof(btif_av_cb.peer_bda)) == 0) {
+      btif_av_connect_req_t* connect_req_p = (btif_av_connect_req_t*)p_data;
+      if (btif_av_cb.peer_bda == *connect_req_p->target_bda) {
         BTIF_TRACE_DEBUG(
             "%s: Same device moved to Opening state,ignore Connect Req",
             __func__);
-        btif_queue_advance();
-        break;
       } else {
         BTIF_TRACE_DEBUG("%s: Moved from idle by Incoming Connection request",
                          __func__);
         btif_report_connection_state(
             BTAV_CONNECTION_STATE_DISCONNECTED,
             ((btif_av_connect_req_t*)p_data)->target_bda);
-        btif_queue_advance();
-        break;
       }
+      btif_queue_advance();
+    } break;
 
     case BTA_AV_PENDING_EVT:
       // Check for device, if same device which moved to opening then ignore
@@ -869,9 +865,9 @@ static bool btif_av_state_opened_handler(btif_sm_event_t event, void* p_data) {
       }
       break;
 
-    case BTIF_AV_CONNECT_REQ_EVT:
-      if (memcmp((RawAddress*)p_data, &(btif_av_cb.peer_bda),
-                 sizeof(btif_av_cb.peer_bda)) == 0) {
+    case BTIF_AV_CONNECT_REQ_EVT: {
+      btif_av_connect_req_t* connect_req_p = (btif_av_connect_req_t*)p_data;
+      if (btif_av_cb.peer_bda == *connect_req_p->target_bda) {
         BTIF_TRACE_DEBUG("%s: Ignore BTIF_AV_CONNECT_REQ_EVT for same device",
                          __func__);
       } else {
@@ -881,7 +877,7 @@ static bool btif_av_state_opened_handler(btif_sm_event_t event, void* p_data) {
                                      (RawAddress*)p_data);
       }
       btif_queue_advance();
-      break;
+    } break;
 
     case BTIF_AV_OFFLOAD_START_REQ_EVT:
       BTIF_TRACE_ERROR(

diff --git a/btif/src/btif_core.cc b/btif/src/btif_core.cc
index 43da993..1ac2ae6 100644 (file)
--- a/btif/src/btif_core.cc
+++ b/btif/src/btif_core.cc
@@ -992,7 +992,7 @@ bt_status_t btif_set_adapter_property(const bt_property_t* property) {
   if (storage_req_id != BTIF_CORE_STORAGE_NO_ACTION) {
     /* pass on to storage for updating local database */
 
-    memset(&(req.write_req.bd_addr), 0, sizeof(RawAddress));
+    req.write_req.bd_addr = RawAddress::kEmpty;
     memcpy(&(req.write_req.prop), property, sizeof(bt_property_t));
 
     return btif_transfer_context(execute_storage_request, storage_req_id,
@@ -1019,7 +1019,7 @@ bt_status_t btif_get_remote_device_property(RawAddress* remote_addr,
 
   if (!btif_is_enabled()) return BT_STATUS_NOT_READY;
 
-  memcpy(&(req.read_req.bd_addr), remote_addr, sizeof(RawAddress));
+  req.read_req.bd_addr = *remote_addr;
   req.read_req.type = type;
   return btif_transfer_context(execute_storage_remote_request,
                                BTIF_CORE_STORAGE_REMOTE_READ, (char*)&req,
@@ -1040,7 +1040,7 @@ bt_status_t btif_get_remote_device_properties(RawAddress* remote_addr) {
 
   if (!btif_is_enabled()) return BT_STATUS_NOT_READY;
 
-  memcpy(&(req.read_req.bd_addr), remote_addr, sizeof(RawAddress));
+  req.read_req.bd_addr = *remote_addr;
   return btif_transfer_context(execute_storage_remote_request,
                                BTIF_CORE_STORAGE_REMOTE_READ_ALL, (char*)&req,
                                sizeof(btif_storage_req_t), NULL);

diff --git a/btif/src/btif_profile_queue.cc b/btif/src/btif_profile_queue.cc
index bdf7fc3..4d2efef 100644 (file)
--- a/btif/src/btif_profile_queue.cc
+++ b/btif/src/btif_profile_queue.cc
@@ -122,7 +122,7 @@ bt_status_t btif_queue_connect(uint16_t uuid, const RawAddress* bda,
                                btif_connect_cb_t connect_cb) {
   connect_node_t node;
   memset(&node, 0, sizeof(connect_node_t));
-  memcpy(&node.bda, bda, sizeof(RawAddress));
+  node.bda = *bda;
   node.uuid = uuid;
   node.connect_cb = connect_cb;
 

diff --git a/service/low_energy_client.h b/service/low_energy_client.h
index ea048bb..16ef4a5 100644 (file)
--- a/service/low_energy_client.h
+++ b/service/low_energy_client.h
@@ -35,7 +35,7 @@ namespace bluetooth {
 
 struct ConnComparator {
   bool operator()(const RawAddress& a, const RawAddress& b) const {
-    return memcmp(&a, &b, sizeof(RawAddress)) < 0;
+    return memcmp(a.address, b.address, RawAddress::kLength) < 0;
   }
 };
 

diff --git a/stack/btm/btm_sec.cc b/stack/btm/btm_sec.cc
index bf72171..263db46 100644 (file)
--- a/stack/btm/btm_sec.cc
+++ b/stack/btm/btm_sec.cc
@@ -4984,7 +4984,7 @@ void btm_sec_pin_code_request(const RawAddress& p_bda) {
       btsnd_hcic_pin_code_neg_reply(p_bda);
       return;
     } else if ((btm_cb.pairing_state != BTM_PAIR_STATE_WAIT_PIN_REQ) ||
-               p_bda == btm_cb.pairing_bda) {
+               p_bda != btm_cb.pairing_bda) {
       BTM_TRACE_WARNING("btm_sec_pin_code_request() rejected - state: %s",
                         btm_pair_state_descr(btm_cb.pairing_state));
       btsnd_hcic_pin_code_neg_reply(p_bda);

diff --git a/stack/gatt/gatt_sr.cc b/stack/gatt/gatt_sr.cc
index 37774d8..211e57c 100644 (file)
--- a/stack/gatt/gatt_sr.cc
+++ b/stack/gatt/gatt_sr.cc
@@ -644,6 +644,7 @@ void gatts_process_primary_service_req(tGATT_TCB& tcb, uint8_t op_code,
   if (reason != GATT_SUCCESS) {
     osi_free(p_msg);
     gatt_send_error_rsp(tcb, reason, op_code, s_hdl, false);
+    return;
   }
 
   attp_send_sr_msg(tcb, p_msg);

diff --git a/stack/l2cap/l2c_api.cc b/stack/l2cap/l2c_api.cc
index b921f51..6e3fd3c 100644 (file)
--- a/stack/l2cap/l2c_api.cc
+++ b/stack/l2cap/l2c_api.cc
@@ -1070,7 +1070,7 @@ bool L2CA_SetIdleTimeoutByBdAddr(const RawAddress& bd_addr, uint16_t timeout,
                                  tBT_TRANSPORT transport) {
   tL2C_LCB* p_lcb;
 
-  if (RawAddress::kAny == bd_addr) {
+  if (RawAddress::kAny != bd_addr) {
     p_lcb = l2cu_find_lcb_by_bd_addr(bd_addr, transport);
     if ((p_lcb) && (p_lcb->in_use) && (p_lcb->link_state == LST_CONNECTED)) {
       p_lcb->idle_timeout = timeout;
@@ -1422,7 +1422,7 @@ bool L2CA_SetFlushTimeout(const RawAddress& bd_addr, uint16_t flush_tout) {
     }
   }
 
-  if (RawAddress::kAny == bd_addr) {
+  if (RawAddress::kAny != bd_addr) {
     p_lcb = l2cu_find_lcb_by_bd_addr(bd_addr, BT_TRANSPORT_BR_EDR);
 
     if ((p_lcb) && (p_lcb->in_use) && (p_lcb->link_state == LST_CONNECTED)) {

diff --git a/test/suite/rfcomm/rfcomm_unittest.cc b/test/suite/rfcomm/rfcomm_unittest.cc
index cb9cfe7..eabe73c 100644 (file)
--- a/test/suite/rfcomm/rfcomm_unittest.cc
+++ b/test/suite/rfcomm/rfcomm_unittest.cc
@@ -49,7 +49,7 @@ TEST_F(RFCommTest, RfcommConnectPairedDevice) {
   EXPECT_TRUE(len == sizeof(signal))
       << "Connection signal not read from RFCOMM socket. Bytes read: " << len;
 
-  EXPECT_TRUE(!memcmp(&signal.bd_addr, &bt_remote_bdaddr_, sizeof(RawAddress)))
+  EXPECT_TRUE(signal.bd_addr == bt_remote_bdaddr_)
       << "Connected to a different bdaddr than expected.";
   EXPECT_TRUE(channel == signal.channel)
       << "Inconsistent channels returned: " << channel << " and "
@@ -99,8 +99,7 @@ TEST_F(RFCommTest, RfcommRepeatedConnectPairedDevice) {
       signal_fail++;
     }
 
-    EXPECT_TRUE(
-        !memcmp(&signal.bd_addr, &bt_remote_bdaddr_, sizeof(RawAddress)))
+    EXPECT_TRUE(signal.bd_addr == bt_remote_bdaddr_)
         << "Connected to a different bdaddr than expected.";
     EXPECT_TRUE(channel == signal.channel)
         << "Inconsistent channels returned: " << channel << " and "