Update the usage of fixed_queue.
* 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
diff --git a/system/osi/include/fixed_queue.h b/system/osi/include/fixed_queue.h
index 6902d6b..bb066f8 100644
--- a/system/osi/include/fixed_queue.h
+++ b/system/osi/include/fixed_queue.h
@@ -45,11 +45,12 @@
// 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 @@
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
diff --git a/system/osi/src/fixed_queue.c b/system/osi/src/fixed_queue.c
index 03a252c..5db43a5 100644
--- a/system/osi/src/fixed_queue.c
+++ b/system/osi/src/fixed_queue.c
@@ -116,7 +116,8 @@
}
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 @@
}
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 @@
}
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_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_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_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 @@
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;
}
diff --git a/system/stack/avdt/avdt_scb_act.c b/system/stack/avdt/avdt_scb_act.c
index fd5ba83..de072b3 100644
--- a/system/stack/avdt/avdt_scb_act.c
+++ b/system/stack/avdt/avdt_scb_act.c
@@ -1287,51 +1287,53 @@
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);
+ 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;
+ /* 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;
+ /* 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++;
- }
+ /* 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);
+ 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);
+ /* 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);
+ /* length of all remaining transport packet */
+ UINT16_TO_BE_STREAM(p, p_frag->layer_specific);
+ }
}
/* store it */
diff --git a/system/stack/btm/btm_sec.c b/system/stack/btm/btm_sec.c
index fa29465..a31d6035 100644
--- a/system/stack/btm/btm_sec.c
+++ b/system/stack/btm/btm_sec.c
@@ -6231,8 +6231,10 @@
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);
diff --git a/system/stack/gatt/gatt_sr.c b/system/stack/gatt/gatt_sr.c
index b44d82d..9be12aa 100644
--- a/system/stack/gatt/gatt_sr.c
+++ b/system/stack/gatt/gatt_sr.c
@@ -166,18 +166,22 @@
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)
{
diff --git a/system/stack/gatt/gatt_utils.c b/system/stack/gatt/gatt_utils.c
index b73b6ce..fb09704 100644
--- a/system/stack/gatt/gatt_utils.c
+++ b/system/stack/gatt/gatt_utils.c
@@ -167,6 +167,9 @@
{
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 *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 @@
{
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 @@
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)) {
diff --git a/system/stack/l2cap/l2c_fcr.c b/system/stack/l2cap/l2c_fcr.c
index 6554d9a..d6cfcec 100644
--- a/system/stack/l2cap/l2c_fcr.c
+++ b/system/stack/l2cap/l2c_fcr.c
@@ -1511,25 +1511,31 @@
}
/* 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 @@
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 @@
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;
+ 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 )
- {
- /* 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)
+ 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;
+
+ 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;
}
-
- 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;
}
}
