OSDN Git Service

Update the usage of fixed_queue.
authorPavlin Radoslavov <pavlin@google.com>
Thu, 8 Oct 2015 01:07:48 +0000 (18:07 -0700)
committerPavlin Radoslavov <pavlin@google.com>
Thu, 8 Oct 2015 09:20:03 +0000 (09:20 +0000)
 * Relax non-NULL requirements for fixed_queue by eliminating
   some of the asserts.
   Now, when semantically possible, fixed_queue - related function
   will return the appropriate value even if the queue pointer is NULL.
   This reduces clutter in the code where we had to do anyway
   "if (queue != NULL)" checks.

 * Add non-NULL guards in the few remaining places where
   fixed_queue_get_list(). For now, we need to use this function,
   hence the extra check.
   That function should be eliminated in the future, because all the
   code where it is used violates the semantics of using a queue.

Bug: 24723840
Change-Id: I47632a3515f3d27856d4870e18723d345c040d64

osi/include/fixed_queue.h
osi/src/fixed_queue.c
stack/avdt/avdt_scb_act.c
stack/btm/btm_sec.c
stack/gatt/gatt_sr.c
stack/gatt/gatt_utils.c
stack/l2cap/l2c_fcr.c

index 6902d6b..bb066f8 100644 (file)
@@ -45,11 +45,12 @@ fixed_queue_t *fixed_queue_new(size_t capacity);
 // blocked on it) results in undefined behaviour.
 void fixed_queue_free(fixed_queue_t *queue, fixed_queue_free_cb free_cb);
 
-// Returns a value indicating whether the given |queue| is empty. |queue| may
-// not be NULL.
+// Returns a value indicating whether the given |queue| is empty. If |queue|
+// is NULL, the return value is true.
 bool fixed_queue_is_empty(fixed_queue_t *queue);
 
-// Returns the length of the |queue|. |queue| may not be NULL.
+// Returns the length of the |queue|. If |queue| is NULL, the return value
+// is 0.
 size_t fixed_queue_length(fixed_queue_t *queue);
 
 // Returns the maximum number of elements this queue may hold. |queue| may
@@ -73,19 +74,18 @@ void *fixed_queue_dequeue(fixed_queue_t *queue);
 bool fixed_queue_try_enqueue(fixed_queue_t *queue, void *data);
 
 // Tries to dequeue an element from |queue|. This function will never block
-// the caller. If the queue is empty, this function returns NULL immediately.
-// Otherwise, the next element in the queue is returned. |queue| may not be
-// NULL.
+// the caller. If the queue is empty or NULL, this function returns NULL
+// immediately. Otherwise, the next element in the queue is returned.
 void *fixed_queue_try_dequeue(fixed_queue_t *queue);
 
 // Returns the first element from |queue|, if present, without dequeuing it.
 // This function will never block the caller. Returns NULL if there are no
-// elements in the queue. |queue| may not be NULL.
+// elements in the queue or |queue| is NULL.
 void *fixed_queue_try_peek_first(fixed_queue_t *queue);
 
 // Returns the last element from |queue|, if present, without dequeuing it.
 // This function will never block the caller. Returns NULL if there are no
-// elements in the queue. |queue| may not be NULL.
+// elements in the queue or |queue| is NULL.
 void *fixed_queue_try_peek_last(fixed_queue_t *queue);
 
 // Tries to remove a |data| element from the middle of the |queue|. This
index 03a252c..5db43a5 100644 (file)
@@ -116,7 +116,8 @@ void fixed_queue_free(fixed_queue_t *queue, fixed_queue_free_cb free_cb) {
 }
 
 bool fixed_queue_is_empty(fixed_queue_t *queue) {
-  assert(queue != NULL);
+  if (queue == NULL)
+    return true;
 
   pthread_mutex_lock(&queue->lock);
   bool is_empty = list_is_empty(queue->list);
@@ -126,7 +127,8 @@ bool fixed_queue_is_empty(fixed_queue_t *queue) {
 }
 
 size_t fixed_queue_length(fixed_queue_t *queue) {
-  assert(queue != NULL);
+  if (queue == NULL)
+    return 0;
 
   pthread_mutex_lock(&queue->lock);
   size_t length = list_length(queue->list);
@@ -185,7 +187,8 @@ bool fixed_queue_try_enqueue(fixed_queue_t *queue, void *data) {
 }
 
 void *fixed_queue_try_dequeue(fixed_queue_t *queue) {
-  assert(queue != NULL);
+  if (queue == NULL)
+    return NULL;
 
   if (!semaphore_try_wait(queue->dequeue_sem))
     return NULL;
@@ -201,7 +204,8 @@ void *fixed_queue_try_dequeue(fixed_queue_t *queue) {
 }
 
 void *fixed_queue_try_peek_first(fixed_queue_t *queue) {
-  assert(queue != NULL);
+  if (queue == NULL)
+    return NULL;
 
   pthread_mutex_lock(&queue->lock);
   void *ret = list_is_empty(queue->list) ? NULL : list_front(queue->list);
@@ -211,7 +215,8 @@ void *fixed_queue_try_peek_first(fixed_queue_t *queue) {
 }
 
 void *fixed_queue_try_peek_last(fixed_queue_t *queue) {
-  assert(queue != NULL);
+  if (queue == NULL)
+    return NULL;
 
   pthread_mutex_lock(&queue->lock);
   void *ret = list_is_empty(queue->list) ? NULL : list_back(queue->list);
@@ -221,7 +226,8 @@ void *fixed_queue_try_peek_last(fixed_queue_t *queue) {
 }
 
 void *fixed_queue_try_remove_from_queue(fixed_queue_t *queue, void *data) {
-  assert(queue != NULL);
+  if (queue == NULL)
+    return NULL;
 
   pthread_mutex_lock(&queue->lock);
   bool removed = list_remove(queue->list, data);
@@ -236,7 +242,7 @@ list_t *fixed_queue_get_list(fixed_queue_t *queue) {
   assert(queue != NULL);
 
   // NOTE: This function is not thread safe, and there is no point for
-  // callint pthread_mutex_lock() / pthread_mutex_unlock()
+  // calling pthread_mutex_lock() / pthread_mutex_unlock()
   return queue->list;
 }
 
index fd5ba83..de072b3 100644 (file)
@@ -1287,51 +1287,53 @@ void avdt_scb_hdl_write_req_frag(tAVDT_SCB *p_scb, tAVDT_SCB_EVT *p_data)
 
     ssrc = avdt_scb_gen_ssrc(p_scb);
 
-    list_t *list = fixed_queue_get_list(p_data->apiwrite.frag_q);
-    const list_node_t *node = list_begin(list);
-    if (node != list_end(list)) {
-        p_frag = (BT_HDR *)list_node(node);
-        node = list_next(node);
-
-        /* get first packet */
-        /* posit on Adaptation Layer header */
-        p_frag->len += AVDT_AL_HDR_SIZE + AVDT_MEDIA_HDR_SIZE;
-        p_frag->offset -= AVDT_AL_HDR_SIZE + AVDT_MEDIA_HDR_SIZE;
-        p = (UINT8 *)(p_frag + 1) + p_frag->offset;
-
-        /* Adaptation Layer header */
-        /* TSID, no-fragment bit and coding of length (in 2 length octets
-         * following)
-         */
-        *p++ = (p_scb->curr_cfg.mux_tsid_media<<3) | AVDT_ALH_LCODE_16BIT;
-
-        /* length of all remaining transport packet */
-        UINT16_TO_BE_STREAM(p, p_frag->layer_specific + AVDT_MEDIA_HDR_SIZE );
-        /* media header */
-        UINT8_TO_BE_STREAM(p, AVDT_MEDIA_OCTET1);
-        UINT8_TO_BE_STREAM(p, p_data->apiwrite.m_pt);
-        UINT16_TO_BE_STREAM(p, p_scb->media_seq);
-        UINT32_TO_BE_STREAM(p, p_data->apiwrite.time_stamp);
-        UINT32_TO_BE_STREAM(p, ssrc);
-        p_scb->media_seq++;
-    }
+    if (! fixed_queue_is_empty(p_data->apiwrite.frag_q)) {
+        list_t *list = fixed_queue_get_list(p_data->apiwrite.frag_q);
+        const list_node_t *node = list_begin(list);
+        if (node != list_end(list)) {
+            p_frag = (BT_HDR *)list_node(node);
+            node = list_next(node);
+
+            /* get first packet */
+            /* posit on Adaptation Layer header */
+            p_frag->len += AVDT_AL_HDR_SIZE + AVDT_MEDIA_HDR_SIZE;
+            p_frag->offset -= AVDT_AL_HDR_SIZE + AVDT_MEDIA_HDR_SIZE;
+            p = (UINT8 *)(p_frag + 1) + p_frag->offset;
+
+            /* Adaptation Layer header */
+            /* TSID, no-fragment bit and coding of length (in 2 length octets
+             * following)
+             */
+            *p++ = (p_scb->curr_cfg.mux_tsid_media<<3) | AVDT_ALH_LCODE_16BIT;
+
+            /* length of all remaining transport packet */
+            UINT16_TO_BE_STREAM(p, p_frag->layer_specific + AVDT_MEDIA_HDR_SIZE );
+            /* media header */
+            UINT8_TO_BE_STREAM(p, AVDT_MEDIA_OCTET1);
+            UINT8_TO_BE_STREAM(p, p_data->apiwrite.m_pt);
+            UINT16_TO_BE_STREAM(p, p_scb->media_seq);
+            UINT32_TO_BE_STREAM(p, p_data->apiwrite.time_stamp);
+            UINT32_TO_BE_STREAM(p, ssrc);
+            p_scb->media_seq++;
+        }
 
-    for ( ; node != list_end(list); node = list_next(node)) {
-        p_frag = (BT_HDR *)list_node(node);
-
-        /* posit on Adaptation Layer header */
-        p_frag->len += AVDT_AL_HDR_SIZE;
-        p_frag->offset -= AVDT_AL_HDR_SIZE;
-        p = (UINT8 *)(p_frag + 1) + p_frag->offset;
-        /* Adaptation Layer header */
-        /* TSID, fragment bit and coding of length (in 2 length octets
-         * following)
-         */
-        *p++ = (p_scb->curr_cfg.mux_tsid_media << 3) |
-            (AVDT_ALH_FRAG_MASK | AVDT_ALH_LCODE_16BIT);
-
-        /* length of all remaining transport packet */
-        UINT16_TO_BE_STREAM(p, p_frag->layer_specific);
+        for ( ; node != list_end(list); node = list_next(node)) {
+            p_frag = (BT_HDR *)list_node(node);
+
+            /* posit on Adaptation Layer header */
+            p_frag->len += AVDT_AL_HDR_SIZE;
+            p_frag->offset -= AVDT_AL_HDR_SIZE;
+            p = (UINT8 *)(p_frag + 1) + p_frag->offset;
+            /* Adaptation Layer header */
+            /* TSID, fragment bit and coding of length (in 2 length octets
+             * following)
+             */
+            *p++ = (p_scb->curr_cfg.mux_tsid_media << 3) |
+                (AVDT_ALH_FRAG_MASK | AVDT_ALH_LCODE_16BIT);
+
+            /* length of all remaining transport packet */
+            UINT16_TO_BE_STREAM(p, p_frag->layer_specific);
+        }
     }
 
     /* store it */
index fa29465..a31d603 100644 (file)
@@ -6231,8 +6231,10 @@ static BOOLEAN btm_sec_is_serv_level0(UINT16 psm)
 static void btm_sec_check_pending_enc_req (tBTM_SEC_DEV_REC  *p_dev_rec, tBT_TRANSPORT transport,
                                             UINT8 encr_enable)
 {
-    UINT8                   res = encr_enable ? BTM_SUCCESS : BTM_ERR_PROCESSING;
+    if (fixed_queue_is_empty(btm_cb.sec_pending_q))
+        return;
 
+    UINT8 res = encr_enable ? BTM_SUCCESS : BTM_ERR_PROCESSING;
     list_t *list = fixed_queue_get_list(btm_cb.sec_pending_q);
     for (const list_node_t *node = list_begin(list); node != list_end(list); ) {
         tBTM_SEC_QUEUE_ENTRY *p_e = (tBTM_SEC_QUEUE_ENTRY *)list_node(node);
index b44d82d..9be12aa 100644 (file)
@@ -166,18 +166,22 @@ static BOOLEAN process_read_multi_rsp (tGATT_SR_CMD *p_cmd, tGATT_STATUS status,
             p_buf->len = 1;
 
             /* Now walk through the buffers puting the data into the response in order */
-            list_t *list = fixed_queue_get_list(p_cmd->multi_rsp_q);
-            const list_node_t *node;
+            list_t *list = NULL;
+            const list_node_t *node = NULL;
+            if (! fixed_queue_is_empty(p_cmd->multi_rsp_q))
+                list = fixed_queue_get_list(p_cmd->multi_rsp_q);
             for (ii = 0; ii < p_cmd->multi_req.num_handles; ii++)
             {
                 tGATTS_RSP *p_rsp = NULL;
 
-                if (ii == 0)
-                    node = list_begin(list);
-                else
-                    node = list_next(node);
-                if (node != list_end(list))
-                    p_rsp = (tGATTS_RSP *)list_node(node);
+                if (list != NULL) {
+                    if (ii == 0)
+                        node = list_begin(list);
+                    else
+                        node = list_next(node);
+                    if (node != list_end(list))
+                        p_rsp = (tGATTS_RSP *)list_node(node);
+                }
 
                 if (p_rsp != NULL)
                 {
index b73b6ce..fb09704 100644 (file)
@@ -167,6 +167,9 @@ void gatt_set_srv_chg(void)
 {
     GATT_TRACE_DEBUG ("gatt_set_srv_chg");
 
+    if (fixed_queue_is_empty(gatt_cb.srv_chg_clt_q))
+        return;
+
     list_t *list = fixed_queue_get_list(gatt_cb.srv_chg_clt_q);
     for (const list_node_t *node = list_begin(list); node != list_end(list);
          node = list_next(node)) {
@@ -198,6 +201,9 @@ tGATTS_PENDING_NEW_SRV_START *gatt_sr_is_new_srv_chg(tBT_UUID *p_app_uuid128, tB
 {
     tGATTS_PENDING_NEW_SRV_START *p_buf = NULL;
 
+    if (fixed_queue_is_empty(gatt_cb.pending_new_srv_start_q))
+        return NULL;
+
     list_t *list = fixed_queue_get_list(gatt_cb.pending_new_srv_start_q);
     for (const list_node_t *node = list_begin(list); node != list_end(list);
          node = list_next(node)) {
@@ -771,7 +777,7 @@ BOOLEAN gatt_is_srv_chg_ind_pending (tGATT_TCB *p_tcb)
     {
         srv_chg_ind_pending = TRUE;
     }
-    else
+    else if (! fixed_queue_is_empty(p_tcb->pending_ind_q))
     {
         list_t *list = fixed_queue_get_list(p_tcb->pending_ind_q);
         for (const list_node_t *node = list_begin(list);
@@ -807,6 +813,9 @@ tGATTS_SRV_CHG *gatt_is_bda_in_the_srv_chg_clt_list (BD_ADDR bda)
     GATT_TRACE_DEBUG("gatt_is_bda_in_the_srv_chg_clt_list :%02x-%02x-%02x-%02x-%02x-%02x",
                       bda[0],  bda[1], bda[2],  bda[3], bda[4],  bda[5]);
 
+    if (fixed_queue_is_empty(gatt_cb.srv_chg_clt_q))
+        return NULL;
+
     list_t *list = fixed_queue_get_list(gatt_cb.srv_chg_clt_q);
     for (const list_node_t *node = list_begin(list); node != list_end(list);
          node = list_next(node)) {
index 6554d9a..d6cfcec 100644 (file)
@@ -1511,25 +1511,31 @@ static BOOLEAN retransmit_i_frames (tL2C_CCB *p_ccb, UINT8 tx_seq)
     }
 
     /* tx_seq indicates whether to retransmit a specific sequence or all (if == L2C_FCR_RETX_ALL_PKTS) */
-    list_t *list_ack = fixed_queue_get_list(p_ccb->fcrb.waiting_for_ack_q);
-    const list_node_t *node_ack = list_begin(list_ack);
+    list_t *list_ack = NULL;
+    const list_node_t *node_ack = NULL;
+    if (! fixed_queue_is_empty(p_ccb->fcrb.waiting_for_ack_q)) {
+        list_ack = fixed_queue_get_list(p_ccb->fcrb.waiting_for_ack_q);
+        node_ack = list_begin(list_ack);
+    }
     if (tx_seq != L2C_FCR_RETX_ALL_PKTS)
     {
         /* If sending only one, the sequence number tells us which one. Look for it.
         */
-        for ( ; node_ack != list_end(list_ack); node_ack = list_next(node_ack)) {
-            p_buf = (BT_HDR *)list_node(node_ack);
-            /* Get the old control word */
-            p = ((UINT8 *) (p_buf+1)) + p_buf->offset + L2CAP_PKT_OVERHEAD;
+        if (list_ack != NULL) {
+            for ( ; node_ack != list_end(list_ack); node_ack = list_next(node_ack)) {
+                p_buf = (BT_HDR *)list_node(node_ack);
+                /* Get the old control word */
+                p = ((UINT8 *) (p_buf+1)) + p_buf->offset + L2CAP_PKT_OVERHEAD;
 
-            STREAM_TO_UINT16 (ctrl_word, p);
+                STREAM_TO_UINT16 (ctrl_word, p);
 
-            buf_seq = (ctrl_word & L2CAP_FCR_TX_SEQ_BITS) >> L2CAP_FCR_TX_SEQ_BITS_SHIFT;
+                buf_seq = (ctrl_word & L2CAP_FCR_TX_SEQ_BITS) >> L2CAP_FCR_TX_SEQ_BITS_SHIFT;
 
-            L2CAP_TRACE_DEBUG ("retransmit_i_frames()   cur seq: %u  looking for: %u", buf_seq, tx_seq);
+                L2CAP_TRACE_DEBUG ("retransmit_i_frames()   cur seq: %u  looking for: %u", buf_seq, tx_seq);
 
-            if (tx_seq == buf_seq)
-                break;
+                if (tx_seq == buf_seq)
+                    break;
+            }
         }
 
         if (!p_buf)
@@ -1560,24 +1566,27 @@ static BOOLEAN retransmit_i_frames (tL2C_CCB *p_ccb, UINT8 tx_seq)
         while (!fixed_queue_is_empty(p_ccb->fcrb.retrans_q))
             osi_freebuf(fixed_queue_try_dequeue(p_ccb->fcrb.retrans_q));
 
-        node_ack = list_begin(list_ack);
+        if (list_ack != NULL)
+            node_ack = list_begin(list_ack);
     }
 
-    while (node_ack != list_end(list_ack))
-    {
-        p_buf = (BT_HDR *)list_node(node_ack);
-        node_ack = list_next(node_ack);
-
-        BT_HDR *p_buf2 = l2c_fcr_clone_buf(p_buf, p_buf->offset, p_buf->len);
-        if (p_buf2)
+    if (list_ack != NULL) {
+        while (node_ack != list_end(list_ack))
         {
-            p_buf2->layer_specific = p_buf->layer_specific;
+            p_buf = (BT_HDR *)list_node(node_ack);
+            node_ack = list_next(node_ack);
 
-            fixed_queue_enqueue(p_ccb->fcrb.retrans_q, p_buf2);
-        }
+            BT_HDR *p_buf2 = l2c_fcr_clone_buf(p_buf, p_buf->offset, p_buf->len);
+            if (p_buf2)
+            {
+                p_buf2->layer_specific = p_buf->layer_specific;
+
+                fixed_queue_enqueue(p_ccb->fcrb.retrans_q, p_buf2);
+            }
 
-        if ( (tx_seq != L2C_FCR_RETX_ALL_PKTS) || (p_buf2 == NULL) )
-            break;
+            if ( (tx_seq != L2C_FCR_RETX_ALL_PKTS) || (p_buf2 == NULL) )
+                break;
+        }
     }
 
     l2c_link_check_send_pkts (p_ccb->p_lcb, NULL, NULL);
@@ -2282,31 +2291,35 @@ static void l2c_fcr_collect_ack_delay (tL2C_CCB *p_ccb, UINT8 num_bufs_acked)
         p_ccb->fcrb.ack_q_count_min[index] = fixed_queue_length(p_ccb->fcrb.waiting_for_ack_q);
 
     /* update sum, max and min of round trip delay of acking */
-    list_t *list = fixed_queue_get_list(p_ccb->fcrb.waiting_for_ack_q);
-    for (const list_node_t *node = list_begin(list), xx = 0;
-         (node != list_end(list)) && (xx < num_bufs_acked);
-         node = list_next(node), xx++) {
-        p_buf = list_node(node);
-        /* adding up length of acked I-frames to get throughput */
-        p_ccb->fcrb.throughput[index] += p_buf->len - 8;
-
-        if ( xx == num_bufs_acked - 1 )
-        {
-            /* get timestamp from tx I-frame that receiver is acking */
-            p = ((UINT8 *) (p_buf+1)) + p_buf->offset + p_buf->len;
-            if (p_ccb->bypass_fcs != L2CAP_BYPASS_FCS)
+    list_t *list = NULL;
+    if (! fixed_queue_is_empty(p_ccb->fcrb.waiting_for_ack_q))
+        list = fixed_queue_get_list(p_ccb->fcrb.waiting_for_ack_q);
+    if (list != NULL) {
+        for (const list_node_t *node = list_begin(list), xx = 0;
+             (node != list_end(list)) && (xx < num_bufs_acked);
+             node = list_next(node), xx++) {
+            p_buf = list_node(node);
+            /* adding up length of acked I-frames to get throughput */
+            p_ccb->fcrb.throughput[index] += p_buf->len - 8;
+
+            if ( xx == num_bufs_acked - 1 )
             {
-                p += L2CAP_FCS_LEN;
-            }
+                /* get timestamp from tx I-frame that receiver is acking */
+                p = ((UINT8 *) (p_buf+1)) + p_buf->offset + p_buf->len;
+                if (p_ccb->bypass_fcs != L2CAP_BYPASS_FCS)
+                {
+                    p += L2CAP_FCS_LEN;
+                }
 
-            STREAM_TO_UINT32(timestamp, p);
-            delay = time_get_os_boottime_ms() - timestamp;
+                STREAM_TO_UINT32(timestamp, p);
+                delay = time_get_os_boottime_ms() - timestamp;
 
-            p_ccb->fcrb.ack_delay_avg[index] += delay;
-            if ( delay > p_ccb->fcrb.ack_delay_max[index] )
-                p_ccb->fcrb.ack_delay_max[index] = delay;
-            if ( delay < p_ccb->fcrb.ack_delay_min[index] )
-                p_ccb->fcrb.ack_delay_min[index] = delay;
+                p_ccb->fcrb.ack_delay_avg[index] += delay;
+                if ( delay > p_ccb->fcrb.ack_delay_max[index] )
+                    p_ccb->fcrb.ack_delay_max[index] = delay;
+                if ( delay < p_ccb->fcrb.ack_delay_min[index] )
+                    p_ccb->fcrb.ack_delay_min[index] = delay;
+            }
         }
     }