Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f7873507 authored by Jack He's avatar Jack He
Browse files

RFCOMM: Improve logging and readability 

Debug Improvements:
* Add more error logging in the following methods:
 - rfc_mx_sm_state_idle for L2CA_ConnectReq failure
 - rfc_mx_sm_state_configure for RFC_EVENT_TIMEOUT
 - rfc_mx_conf_cnf for L2CAP configuration failures
 - RFCOMM_RemoveServer

Readability Improvements:
* Refactored logic in the following methods to be more readable
 - L2CA_ErtmConnectReq
 - RFCOMM_CreateConnection
 - port_open_continue
 - PORT_StartCnf
 - port_allocate_port
 - rfc_send_buf_uih
 - bta_ag_get_other_idle_scb
* Rename RFCOMM_ParNegReq to RFCOMM_ParameterNegotiationRequest
* Rename RFCOMM_ParNegRsp to RFCOMM_ParameterNegotiationResponse
* Rename RFCOMM_PortNegReq to RFCOMM_PortParameterNegotiationRequest
* Rename RFCOMM_PortNegRsp to RFCOMM_PortParameterNegotiationResponse

Rename using IDE:
* Rename tRFCOMM_CB.last_port to tRFCOMM_CB.last_port_index
* Rename tPORT.inx to tPORT.handle as inx is ready the port handle
  indexed from 1
* Rename tRFC_MCB.port_inx to tRFC_MCB.port_handles

NPE Prevention:
* Add error checking in rfc_save_lcid_mcb to avoid accessing beyond
  array boundary

Bug: 77224743
Test: StackRfcommTest, connect and disconnect to multiple devices
      with PBAP, MAP, PAN, PTS HFP tests
      testplans/details/158641/3975
Change-Id: I8de89dfc0cee48fed6ad4e1d4a8cdcde2f960aab
parent 3dfb5f8a

system/bta/ag/bta_ag_act.cc

+18 −29
Original line number Diff line number Diff line
@@ -174,8 +174,6 @@ void bta_ag_start_dereg(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
 *

 ******************************************************************************/

void bta_ag_start_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) {

  RawAddress pending_bd_addr = {};



  /* store parameters */

  if (!data.IsEmpty()) {

    p_scb->peer_addr = data.api_open.bd_addr;
@@ -184,7 +182,8 @@ void bta_ag_start_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
  }



  /* Check if RFCOMM has any incoming connection to avoid collision. */

  if (PORT_IsOpening(pending_bd_addr)) {

  RawAddress pending_bd_addr = RawAddress::kEmpty;

  if (PORT_IsOpening(&pending_bd_addr)) {

    /* Let the incoming connection goes through.                        */

    /* Issue collision for this scb for now.                            */

    /* We will decide what to do when we find incoming connetion later. */
@@ -502,44 +501,34 @@ void bta_ag_rfc_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
 *

 ******************************************************************************/

void bta_ag_rfc_acp_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) {

  uint16_t lcid;

  int i;

  tBTA_AG_SCB *ag_scb, *other_scb;

  RawAddress dev_addr = {};

  int status;



  APPL_TRACE_DEBUG("%s: serv_handle0 = %d serv_handle1 = %d", __func__,

                   p_scb->serv_handle[0], p_scb->serv_handle[1]);

  /* set role */

  p_scb->role = BTA_AG_ACP;



  APPL_TRACE_DEBUG("bta_ag_rfc_acp_open: serv_handle0 = %d serv_handle1 = %d",

                   p_scb->serv_handle[0], p_scb->serv_handle[1]);



  /* get bd addr of peer */

  if (PORT_SUCCESS !=

      (status = PORT_CheckConnection(data.rfc.port_handle, dev_addr, &lcid))) {

    APPL_TRACE_DEBUG(

        "bta_ag_rfc_acp_open error PORT_CheckConnection returned status %d",

        status);

  uint16_t lcid = 0;

  RawAddress dev_addr = RawAddress::kEmpty;

  int status = PORT_CheckConnection(data.rfc.port_handle, &dev_addr, &lcid);

  if (status != PORT_SUCCESS) {

    LOG(ERROR) << __func__ << ", PORT_CheckConnection returned " << status;

  }



  /* Collision Handling */

  for (i = 0, ag_scb = &bta_ag_cb.scb[0]; i < BTA_AG_MAX_NUM_CLIENTS;

       i++, ag_scb++) {

    if (ag_scb->in_use && alarm_is_scheduled(ag_scb->collision_timer)) {

      alarm_cancel(ag_scb->collision_timer);



      if (dev_addr == ag_scb->peer_addr) {

  for (tBTA_AG_SCB& ag_scb : bta_ag_cb.scb) {

    if (ag_scb.in_use && alarm_is_scheduled(ag_scb.collision_timer)) {

      alarm_cancel(ag_scb.collision_timer);

      if (dev_addr == ag_scb.peer_addr) {

        /* If incoming and outgoing device are same, nothing more to do. */

        /* Outgoing conn will be aborted because we have successful incoming

         * conn.  */

      } else {

        /* Resume outgoing connection. */

        other_scb = bta_ag_get_other_idle_scb(p_scb);

        tBTA_AG_SCB* other_scb = bta_ag_get_other_idle_scb(p_scb);

        if (other_scb) {

          other_scb->peer_addr = ag_scb->peer_addr;

          other_scb->open_services = ag_scb->open_services;

          other_scb->cli_sec_mask = ag_scb->cli_sec_mask;



          other_scb->peer_addr = ag_scb.peer_addr;

          other_scb->open_services = ag_scb.open_services;

          other_scb->cli_sec_mask = ag_scb.cli_sec_mask;

          bta_ag_resume_open(other_scb);

        }

      }
@@ -551,7 +540,7 @@ void bta_ag_rfc_acp_open(tBTA_AG_SCB* p_scb, const tBTA_AG_DATA& data) { 
  p_scb->peer_addr = dev_addr;



  /* determine connected service from port handle */

  for (i = 0; i < BTA_AG_NUM_IDX; i++) {

  for (uint8_t i = 0; i < BTA_AG_NUM_IDX; i++) {

    APPL_TRACE_DEBUG(

        "bta_ag_rfc_acp_open: i = %d serv_handle = %d port_handle = %d", i,

        p_scb->serv_handle[i], data.rfc.port_handle);

system/bta/ag/bta_ag_main.cc

+6 −11
Original line number Diff line number Diff line
@@ -505,16 +505,11 @@ bool bta_ag_scb_open(tBTA_AG_SCB* p_curr_scb) { 
 *

 ******************************************************************************/

tBTA_AG_SCB* bta_ag_get_other_idle_scb(tBTA_AG_SCB* p_curr_scb) {

  tBTA_AG_SCB* p_scb = &bta_ag_cb.scb[0];

  uint8_t xx;



  for (xx = 0; xx < BTA_AG_MAX_NUM_CLIENTS; xx++, p_scb++) {

    if (p_scb->in_use && (p_scb != p_curr_scb) &&

        (p_scb->state == BTA_AG_INIT_ST)) {

      return p_scb;

  for (tBTA_AG_SCB& scb : bta_ag_cb.scb) {

    if (scb.in_use && (&scb != p_curr_scb) && (scb.state == BTA_AG_INIT_ST)) {

      return &scb;

    }

  }



  /* no other scb found */

  APPL_TRACE_DEBUG("bta_ag_get_other_idle_scb: No idle AG scb");

  return nullptr;
@@ -559,13 +554,13 @@ void bta_ag_collision_cback(UNUSED_ATTR tBTA_SYS_CONN_STATUS status, uint8_t id, 


  if (p_scb && (p_scb->state == BTA_AG_OPENING_ST)) {

    if (id == BTA_ID_SYS) {

      LOG(WARNING) << __func__ << "AG found collision (ACL) for handle "

      LOG(WARNING) << __func__ << ": AG found collision (ACL) for handle "

                   << unsigned(handle) << " device " << peer_addr;

    } else if (id == BTA_ID_AG) {

      LOG(WARNING) << __func__ << "AG found collision (RFCOMM) for handle "

      LOG(WARNING) << __func__ << ": AG found collision (RFCOMM) for handle "

                   << unsigned(handle) << " device " << peer_addr;

    } else {

      LOG(WARNING) << __func__ << "AG found collision (UNKNOWN) for handle "

      LOG(WARNING) << __func__ << ": AG found collision (UNKNOWN) for handle "

                   << unsigned(handle) << " device " << peer_addr;

    }

system/bta/av/bta_av_aact.cc

+5 −4
Original line number Diff line number Diff line
@@ -264,11 +264,12 @@ static uint8_t bta_av_get_scb_sep_type(tBTA_AV_SCB* p_scb, 
 * Returns          void

 *

 ******************************************************************************/

static void bta_av_save_addr(tBTA_AV_SCB* p_scb, const RawAddress& b) {

static void bta_av_save_addr(tBTA_AV_SCB* p_scb, const RawAddress& bd_addr) {

  APPL_TRACE_DEBUG("%s: r:%d, s:%d", __func__, p_scb->recfg_sup,

                   p_scb->suspend_sup);

  if (p_scb->peer_addr != b) {

    APPL_TRACE_ERROR("%s: reset flags", __func__);

  if (p_scb->peer_addr != bd_addr) {

    LOG(INFO) << __func__ << ": reset flags old_addr=" << p_scb->peer_addr

              << ", new_addr=" << bd_addr;

    /* a new addr, reset the supported flags */

    p_scb->recfg_sup = true;

    p_scb->suspend_sup = true;
@@ -276,7 +277,7 @@ static void bta_av_save_addr(tBTA_AV_SCB* p_scb, const RawAddress& b) { 


  /* do this copy anyway, just in case the first addr matches

   * the control block one by accident */

  p_scb->peer_addr = b;

  p_scb->peer_addr = bd_addr;

}



/*******************************************************************************

system/bta/hf_client/bta_hf_client_act.cc

+7 −11
Original line number Diff line number Diff line
@@ -103,8 +103,8 @@ void bta_hf_client_start_open(tBTA_HF_CLIENT_DATA* p_data) { 
  }



  /* Check if RFCOMM has any incoming connection to avoid collision. */

  RawAddress pending_bd_addr;

  if (PORT_IsOpening(pending_bd_addr)) {

  RawAddress pending_bd_addr = RawAddress::kEmpty;

  if (PORT_IsOpening(&pending_bd_addr)) {

    /* Let the incoming connection goes through.                        */

    /* Issue collision for now.                                         */

    /* We will decide what to do when we find incoming connection later.*/
@@ -164,21 +164,17 @@ void bta_hf_client_rfc_acp_open(tBTA_HF_CLIENT_DATA* p_data) { 
                     p_data->hdr.layer_specific);

    return;

  }



  uint16_t lcid;

  RawAddress dev_addr;

  int status;



  /* set role */

  client_cb->role = BTA_HF_CLIENT_ACP;



  APPL_TRACE_DEBUG("%s: conn_handle %d", __func__, client_cb->conn_handle);



  /* get bd addr of peer */

  if (PORT_SUCCESS != (status = PORT_CheckConnection(client_cb->conn_handle,

                                                     dev_addr, &lcid))) {

    APPL_TRACE_DEBUG("%s: error PORT_CheckConnection returned status %d",

                     __func__, status);

  uint16_t lcid = 0;

  RawAddress dev_addr = RawAddress::kEmpty;

  int status = PORT_CheckConnection(client_cb->conn_handle, &dev_addr, &lcid);

  if (status != PORT_SUCCESS) {

    LOG(ERROR) << __func__ << ": PORT_CheckConnection returned " << status;

  }



  /* Collision Handling */

system/bta/hf_client/bta_hf_client_rfc.cc

+6 −3
Original line number Diff line number Diff line
@@ -98,9 +98,12 @@ static void bta_hf_client_mgmt_cback(uint32_t code, uint16_t port_handle) { 
      APPL_TRACE_DEBUG("%s: allocating a new CB for incoming connection",

                       __func__);

      // Find the BDADDR of the peer device

      RawAddress peer_addr;

      uint16_t lcid;

      PORT_CheckConnection(port_handle, peer_addr, &lcid);

      RawAddress peer_addr = RawAddress::kEmpty;

      uint16_t lcid = 0;

      int status = PORT_CheckConnection(port_handle, &peer_addr, &lcid);

      if (status != PORT_SUCCESS) {

        LOG(ERROR) << __func__ << ": PORT_CheckConnection returned " << status;

      }



      // Since we accepted a remote request we should allocate a handle first.

      uint16_t tmp_handle = -1;