Re: [PATCH] staging: rtl8723au: use list_first_entry*

[Jes Sorensen]

Geliang Tang  writes:
> Use list_first_entry*() instead of container_of() to simplify the code.
>
> Signed-off-by: Geliang Tang 
> ---
>  drivers/staging/rtl8723au/core/rtw_recv.c | 49 
> +--
>  drivers/staging/rtl8723au/core/rtw_xmit.c | 26 +---
>  2 files changed, 22 insertions(+), 53 deletions(-)

This looks fine to me. When these changes gets large, it may be better
to break them down into multiple patches as it's easier to debug if
there is a bug somewhere.


Acked-by: Jes Sorensen 

>
> diff --git a/drivers/staging/rtl8723au/core/rtw_recv.c 
> b/drivers/staging/rtl8723au/core/rtw_recv.c
> index 0a7741c..b095d09 100644
> --- a/drivers/staging/rtl8723au/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723au/core/rtw_recv.c
> @@ -104,21 +104,14 @@ void _rtw_free_recv_priv23a(struct recv_priv *precvpriv)
>  struct recv_frame *rtw_alloc_recvframe23a(struct rtw_queue *pfree_recv_queue)
>  {
>   struct recv_frame *pframe;
> - struct list_head *plist, *phead;
>   struct rtw_adapter *padapter;
>   struct recv_priv *precvpriv;
>  
>   spin_lock_bh(_recv_queue->lock);
>  
> - if (list_empty(_recv_queue->queue))
> - pframe = NULL;
> - else {
> - phead = get_list_head(pfree_recv_queue);
> -
> - plist = phead->next;
> -
> - pframe = container_of(plist, struct recv_frame, list);
> -
> + pframe = list_first_entry_or_null(_recv_queue->queue,
> +   struct recv_frame, list);
> + if (pframe) {
>   list_del_init(>list);
>   padapter = pframe->adapter;
>   if (padapter) {
> @@ -243,25 +236,17 @@ int rtw_enqueue_recvbuf23a(struct recv_buf *precvbuf, 
> struct rtw_queue *queue)
>   return _SUCCESS;
>  }
>  
> -struct recv_buf *rtw_dequeue_recvbuf23a (struct rtw_queue *queue)
> +struct recv_buf *rtw_dequeue_recvbuf23a(struct rtw_queue *queue)
>  {
>   unsigned long irqL;
>   struct recv_buf *precvbuf;
> - struct list_head *plist, *phead;
>  
>   spin_lock_irqsave(>lock, irqL);
>  
> - if (list_empty(>queue)) {
> - precvbuf = NULL;
> - } else {
> - phead = get_list_head(queue);
> -
> - plist = phead->next;
> -
> - precvbuf = container_of(plist, struct recv_buf, list);
> -
> + precvbuf = list_first_entry_or_null(>queue,
> + struct recv_buf, list);
> + if (precvbuf)
>   list_del_init(>list);
> - }
>  
>   spin_unlock_irqrestore(>lock, irqL);
>  
> @@ -1079,22 +1064,17 @@ static int validate_recv_ctrl_frame(struct 
> rtw_adapter *padapter,
>  
>   if ((psta->state & WIFI_SLEEP_STATE) &&
>   (pstapriv->sta_dz_bitmap & CHKBIT(psta->aid))) {
> - struct list_head *xmitframe_plist, *xmitframe_phead;
> + struct list_head *xmitframe_phead;
>   struct xmit_frame *pxmitframe;
>   struct xmit_priv *pxmitpriv = >xmitpriv;
>  
>   spin_lock_bh(>lock);
>  
>   xmitframe_phead = get_list_head(>sleep_q);
> - xmitframe_plist = xmitframe_phead->next;
> -
> - if (!list_empty(xmitframe_phead)) {
> - pxmitframe = container_of(xmitframe_plist,
> -   struct xmit_frame,
> -   list);
> -
> - xmitframe_plist = xmitframe_plist->next;
> -
> + pxmitframe = list_first_entry_or_null(xmitframe_phead,
> +   struct xmit_frame,
> +   list);
> + if (pxmitframe) {
>   list_del_init(>list);
>  
>   psta->sleepq_len--;
> @@ -1542,7 +1522,7 @@ struct recv_frame *recvframe_defrag(struct rtw_adapter 
> *adapter,
>  struct recv_frame *recvframe_defrag(struct rtw_adapter *adapter,
>   struct rtw_queue *defrag_q)
>  {
> - struct list_head *plist, *phead;
> + struct list_head *phead;
>   u8 wlanhdr_offset;
>   u8 curfragnum;
>   struct recv_frame *pnfhdr, *ptmp;
> @@ -1554,8 +1534,7 @@ struct recv_frame *recvframe_defrag(struct rtw_adapter 
> *adapter,
>   pfree_recv_queue = >recvpriv.free_recv_queue;
>  
>   phead = get_list_head(defrag_q);
> - plist = phead->next;
> - prframe = container_of(plist, struct recv_frame, list);
> + prframe = list_first_entry(phead, struct recv_frame, list);
>   list_del_init(>list);
>   skb = prframe->pkt;
>  
> diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c 
> b/drivers/staging/rtl8723au/core/rtw_xmit.c
> index 

[PATCH] staging: rtl8723au: use list_first_entry*

[Geliang Tang]

Use list_first_entry*() instead of container_of() to simplify the code.

Signed-off-by: Geliang Tang 
---
 drivers/staging/rtl8723au/core/rtw_recv.c | 49 +--
 drivers/staging/rtl8723au/core/rtw_xmit.c | 26 +---
 2 files changed, 22 insertions(+), 53 deletions(-)

diff --git a/drivers/staging/rtl8723au/core/rtw_recv.c 
b/drivers/staging/rtl8723au/core/rtw_recv.c
index 0a7741c..b095d09 100644
--- a/drivers/staging/rtl8723au/core/rtw_recv.c
+++ b/drivers/staging/rtl8723au/core/rtw_recv.c
@@ -104,21 +104,14 @@ void _rtw_free_recv_priv23a(struct recv_priv *precvpriv)
 struct recv_frame *rtw_alloc_recvframe23a(struct rtw_queue *pfree_recv_queue)
 {
struct recv_frame *pframe;
-   struct list_head *plist, *phead;
struct rtw_adapter *padapter;
struct recv_priv *precvpriv;
 
spin_lock_bh(_recv_queue->lock);
 
-   if (list_empty(_recv_queue->queue))
-   pframe = NULL;
-   else {
-   phead = get_list_head(pfree_recv_queue);
-
-   plist = phead->next;
-
-   pframe = container_of(plist, struct recv_frame, list);
-
+   pframe = list_first_entry_or_null(_recv_queue->queue,
+ struct recv_frame, list);
+   if (pframe) {
list_del_init(>list);
padapter = pframe->adapter;
if (padapter) {
@@ -243,25 +236,17 @@ int rtw_enqueue_recvbuf23a(struct recv_buf *precvbuf, 
struct rtw_queue *queue)
return _SUCCESS;
 }
 
-struct recv_buf *rtw_dequeue_recvbuf23a (struct rtw_queue *queue)
+struct recv_buf *rtw_dequeue_recvbuf23a(struct rtw_queue *queue)
 {
unsigned long irqL;
struct recv_buf *precvbuf;
-   struct list_head *plist, *phead;
 
spin_lock_irqsave(>lock, irqL);
 
-   if (list_empty(>queue)) {
-   precvbuf = NULL;
-   } else {
-   phead = get_list_head(queue);
-
-   plist = phead->next;
-
-   precvbuf = container_of(plist, struct recv_buf, list);
-
+   precvbuf = list_first_entry_or_null(>queue,
+   struct recv_buf, list);
+   if (precvbuf)
list_del_init(>list);
-   }
 
spin_unlock_irqrestore(>lock, irqL);
 
@@ -1079,22 +1064,17 @@ static int validate_recv_ctrl_frame(struct rtw_adapter 
*padapter,
 
if ((psta->state & WIFI_SLEEP_STATE) &&
(pstapriv->sta_dz_bitmap & CHKBIT(psta->aid))) {
-   struct list_head *xmitframe_plist, *xmitframe_phead;
+   struct list_head *xmitframe_phead;
struct xmit_frame *pxmitframe;
struct xmit_priv *pxmitpriv = >xmitpriv;
 
spin_lock_bh(>lock);
 
xmitframe_phead = get_list_head(>sleep_q);
-   xmitframe_plist = xmitframe_phead->next;
-
-   if (!list_empty(xmitframe_phead)) {
-   pxmitframe = container_of(xmitframe_plist,
- struct xmit_frame,
- list);
-
-   xmitframe_plist = xmitframe_plist->next;
-
+   pxmitframe = list_first_entry_or_null(xmitframe_phead,
+ struct xmit_frame,
+ list);
+   if (pxmitframe) {
list_del_init(>list);
 
psta->sleepq_len--;
@@ -1542,7 +1522,7 @@ struct recv_frame *recvframe_defrag(struct rtw_adapter 
*adapter,
 struct recv_frame *recvframe_defrag(struct rtw_adapter *adapter,
struct rtw_queue *defrag_q)
 {
-   struct list_head *plist, *phead;
+   struct list_head *phead;
u8 wlanhdr_offset;
u8 curfragnum;
struct recv_frame *pnfhdr, *ptmp;
@@ -1554,8 +1534,7 @@ struct recv_frame *recvframe_defrag(struct rtw_adapter 
*adapter,
pfree_recv_queue = >recvpriv.free_recv_queue;
 
phead = get_list_head(defrag_q);
-   plist = phead->next;
-   prframe = container_of(plist, struct recv_frame, list);
+   prframe = list_first_entry(phead, struct recv_frame, list);
list_del_init(>list);
skb = prframe->pkt;
 
diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c 
b/drivers/staging/rtl8723au/core/rtw_xmit.c
index b82b182..3de40cf 100644
--- a/drivers/staging/rtl8723au/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
@@ -1443,24 +1443,18 @@ Must be very very cautious...
 */
 static struct xmit_frame *rtw_alloc_xmitframe(struct xmit_priv *pxmitpriv)
 {
-   struct xmit_frame *pxframe = NULL;
-   struct list_head *plist, *phead;
+   struct xmit_frame