[PATCH] staging: rtl8723bs: refactor xmit_xmitframe

[PATCH] staging: rtl8723bs: refactor xmit_xmitframe

[Eric LI (Honggang)]

Refactor the function xmit_xmitframe in rtl8723bs_xmit.c to
reduce the leading tabs

Signed-off-by: Eric LI (Honggang) <eric.lee0305@gmail.com>
---
 .../staging/rtl8723bs/hal/rtl8723bs_xmit.c    | 43 ++++++++++---------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
index 7f55448d544e..46895b05538d 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
@@ -231,29 +231,30 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
 
 				/*  check xmit_buf size enough or not */
 				txlen = txdesc_size + rtw_wlan_pkt_size(pxmitframe);
-				if (!pxmitbuf ||
-					((_RND(pxmitbuf->len, 8) + txlen) > max_xmit_len) ||
-					(k >= (rtw_hal_sdio_max_txoqt_free_space(padapter) - 1))
+				if (pxmitbuf &&
+				    (((_RND(pxmitbuf->len, 8) + txlen) > max_xmit_len) ||
+				     (k >= (rtw_hal_sdio_max_txoqt_free_space(padapter) - 1)))
 				) {
-					if (pxmitbuf) {
-						/* pxmitbuf->priv_data will be NULL, and will crash here */
-						if (pxmitbuf->len > 0 &&
-						    pxmitbuf->priv_data) {
-							struct xmit_frame *pframe;
-
-							pframe = (struct xmit_frame *)pxmitbuf->priv_data;
-							pframe->agg_num = k;
-							pxmitbuf->agg_num = k;
-							rtl8723b_update_txdesc(pframe, pframe->buf_addr);
-							rtw_free_xmitframe(pxmitpriv, pframe);
-							pxmitbuf->priv_data = NULL;
-							enqueue_pending_xmitbuf(pxmitpriv, pxmitbuf);
-							/* can not yield under lock */
-							/* yield(); */
-						} else
-							rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
-					}
+					if (pxmitbuf->len > 0 &&
+					    pxmitbuf->priv_data) {
+						struct xmit_frame *pframe;
+
+						pframe = (struct xmit_frame *)pxmitbuf->priv_data;
+						pframe->agg_num = k;
+						pxmitbuf->agg_num = k;
+						rtl8723b_update_txdesc(pframe, pframe->buf_addr);
+						rtw_free_xmitframe(pxmitpriv, pframe);
+						pxmitbuf->priv_data = NULL;
+						enqueue_pending_xmitbuf(pxmitpriv, pxmitbuf);
+						/* can not yield under lock */
+						/* yield(); */
+					} else
+						rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
+
+					pxmitbuf = NULL;
+				}
 
+				if (!pxmitbuf) {
 					pxmitbuf = rtw_alloc_xmitbuf(pxmitpriv);
 					if (!pxmitbuf) {
 						err = -2;
-- 
2.34.1

Re: [PATCH] staging: rtl8723bs: refactor xmit_xmitframe

[Greg Kroah-Hartman]

On Thu, Sep 24, 2026 at 11:16:06PM +0800, Eric LI (Honggang) wrote:
> Refactor the function xmit_xmitframe in rtl8723bs_xmit.c to
> reduce the leading tabs

Refactor it how?

And is the output the same before/after?

> 
> Signed-off-by: Eric LI (Honggang) <eric.lee0305@gmail.com>
> ---
>  .../staging/rtl8723bs/hal/rtl8723bs_xmit.c    | 43 ++++++++++---------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> index 7f55448d544e..46895b05538d 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> @@ -231,29 +231,30 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
>  
>  				/*  check xmit_buf size enough or not */
>  				txlen = txdesc_size + rtw_wlan_pkt_size(pxmitframe);
> -				if (!pxmitbuf ||
> -					((_RND(pxmitbuf->len, 8) + txlen) > max_xmit_len) ||
> -					(k >= (rtw_hal_sdio_max_txoqt_free_space(padapter) - 1))
> +				if (pxmitbuf &&
> +				    (((_RND(pxmitbuf->len, 8) + txlen) > max_xmit_len) ||
> +				     (k >= (rtw_hal_sdio_max_txoqt_free_space(padapter) - 1)))
>  				) {
> -					if (pxmitbuf) {
> -						/* pxmitbuf->priv_data will be NULL, and will crash here */
> -						if (pxmitbuf->len > 0 &&
> -						    pxmitbuf->priv_data) {
> -							struct xmit_frame *pframe;
> -
> -							pframe = (struct xmit_frame *)pxmitbuf->priv_data;
> -							pframe->agg_num = k;
> -							pxmitbuf->agg_num = k;
> -							rtl8723b_update_txdesc(pframe, pframe->buf_addr);
> -							rtw_free_xmitframe(pxmitpriv, pframe);
> -							pxmitbuf->priv_data = NULL;
> -							enqueue_pending_xmitbuf(pxmitpriv, pxmitbuf);
> -							/* can not yield under lock */
> -							/* yield(); */
> -						} else
> -							rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
> -					}
> +					if (pxmitbuf->len > 0 &&
> +					    pxmitbuf->priv_data) {
> +						struct xmit_frame *pframe;
> +
> +						pframe = (struct xmit_frame *)pxmitbuf->priv_data;
> +						pframe->agg_num = k;
> +						pxmitbuf->agg_num = k;
> +						rtl8723b_update_txdesc(pframe, pframe->buf_addr);
> +						rtw_free_xmitframe(pxmitpriv, pframe);
> +						pxmitbuf->priv_data = NULL;
> +						enqueue_pending_xmitbuf(pxmitpriv, pxmitbuf);
> +						/* can not yield under lock */
> +						/* yield(); */
> +					} else
> +						rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
> +
> +					pxmitbuf = NULL;

This jumped out at me, why add this new line?

thanks,

greg k-h

Re: [PATCH] staging: rtl8723bs: refactor xmit_xmitframe

[Dan Carpenter]

On Thu, Oct 01, 2026 at 11:34:30AM +0200, Greg Kroah-Hartman wrote:
> > +					if (pxmitbuf->len > 0 &&
> > +					    pxmitbuf->priv_data) {
> > +						struct xmit_frame *pframe;
> > +
> > +						pframe = (struct xmit_frame *)pxmitbuf->priv_data;
> > +						pframe->agg_num = k;
> > +						pxmitbuf->agg_num = k;
> > +						rtl8723b_update_txdesc(pframe, pframe->buf_addr);
> > +						rtw_free_xmitframe(pxmitpriv, pframe);
> > +						pxmitbuf->priv_data = NULL;
> > +						enqueue_pending_xmitbuf(pxmitpriv, pxmitbuf);
> > +						/* can not yield under lock */
> > +						/* yield(); */
> > +					} else
> > +						rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
> > +
> > +					pxmitbuf = NULL;
> 
> This jumped out at me, why add this new line?
> 

The patch is correct but the new version is as confusing as heck...
Glad that I'm not the only person who thinks this.  I've been doing
more and more AI coding these days and they say that AI makes you
stupid so I was worried about my own brain health.

regards,
dan carpenter

Re: [PATCH] staging: rtl8723bs: refactor xmit_xmitframe

[Eric Lee]

On Oct 1, 2026, at 5:34 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> Refactor it how?

The previous code handled two cases in one branch
- pxmitbuf is null, or
- pxmitbuf is not null, but is not enough for current pframe.

That causes an additional check of pxmitbuf in that branch.
In my refactoring, I split them.

Firstly, handle the second case. In this case, pxmitbuf is either
enqueued on pending_xmitbuf_queue, or released. In either way,
pxmitbuf is not owning that xmitbuf anymore.

Next, handle the first case. In this case, a new xmitbuf is
allocated for the current pframe.

Setting pxmitbuf to NULL in the second case in order to make sure
allocating a new xmitbuf for the pframe.


On Oct 1, 2026, at 5:34 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> And is the output the same before/after?

The logic is not changed so the output are same.


On Oct 1, 2026, at 5:34 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > + pxmitbuf = NULL;

> This jumped out at me, why add this new line?

Two reasons to add this line:
1. pxmitbuf is not owning that xmitbuf anymore as it is either enqueued
on pending_xmitbuf_queue, or released.
2. a new xmitbuf is expected to be allocated for the current pframe.

So, it is safe to set pxmitbuf to NULL here, and it ensures
a new xmitbuf is allocated for the pframe.