[PATCH] staging: rtl8723bs: refactor rtw_aes_decrypt() to reduce nesting

[PATCH] staging: rtl8723bs: refactor rtw_aes_decrypt() to reduce nesting

[Lin YuChen]

Improve code readability by refactoring rtw_aes_decrypt() to use
guard clauses and early exits. This reduces the maximum indentation
level and flattens the logic flow for key assignment and decryption.

Signed-off-by: Lin YuChen <starpt.official@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 91 ++++++++++---------
 1 file changed, 46 insertions(+), 45 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index b489babe7432..9229e0a1c792 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -1213,69 +1213,70 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
 	pframe = (unsigned char *)((union recv_frame *)precvframe)->u.hdr.rx_data;
 	/* 4 start to encrypt each fragment */
-	if (prxattrib->encrypt == _AES_) {
-		stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
-		if (stainfo) {
-			if (is_multicast_ether_addr(prxattrib->ra)) {
-				static unsigned long start;
-				static u32 no_gkey_bc_cnt;
-				static u32 no_gkey_mc_cnt;
+	if (prxattrib->encrypt != _AES_)
+		goto exit;
 
-				if (!psecuritypriv->binstallGrpkey) {
-					res = _FAIL;
+	stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
+	if (!stainfo) {
+		res = _FAIL;
+		goto exit;
+	}
+	if (is_multicast_ether_addr(prxattrib->ra)) {
+		static unsigned long start;
+		static u32 no_gkey_bc_cnt;
+		static u32 no_gkey_mc_cnt;
 
-					if (start == 0)
-						start = jiffies;
+		if (!psecuritypriv->binstallGrpkey) {
+			res = _FAIL;
 
-					if (is_broadcast_mac_addr(prxattrib->ra))
-						no_gkey_bc_cnt++;
-					else
-						no_gkey_mc_cnt++;
+			if (start == 0)
+				start = jiffies;
 
-					if (jiffies_to_msecs(jiffies - start) > 1000) {
-						if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-							netdev_dbg(padapter->pnetdev,
-								   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-								   FUNC_ADPT_ARG(padapter),
-								   no_gkey_bc_cnt,
-								   no_gkey_mc_cnt);
-						}
-						start = jiffies;
-						no_gkey_bc_cnt = 0;
-						no_gkey_mc_cnt = 0;
-					}
-
-					goto exit;
-				}
+			if (is_broadcast_mac_addr(prxattrib->ra))
+				no_gkey_bc_cnt++;
+			else
+				no_gkey_mc_cnt++;
 
+			if (jiffies_to_msecs(jiffies - start) > 1000) {
 				if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
 					netdev_dbg(padapter->pnetdev,
-						   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-						   FUNC_ADPT_ARG(padapter),
-						   no_gkey_bc_cnt,
+						   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+						   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
 						   no_gkey_mc_cnt);
 				}
-				start = 0;
+				start = jiffies;
 				no_gkey_bc_cnt = 0;
 				no_gkey_mc_cnt = 0;
-
-				prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
-				if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
-					res = _FAIL;
-					goto exit;
-				}
-			} else {
-				prwskey = &stainfo->dot118021x_UncstKey.skey[0];
 			}
 
-			length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
+			goto exit;
+		}
 
-			res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+		if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+			netdev_dbg(padapter->pnetdev,
+				   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+				   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
+				   no_gkey_mc_cnt);
+		}
+		start = 0;
+		no_gkey_bc_cnt = 0;
+		no_gkey_mc_cnt = 0;
 
-		} else {
+		prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index]
+				  .skey;
+		if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
 			res = _FAIL;
+			goto exit;
 		}
+	} else {
+		prwskey = &stainfo->dot118021x_UncstKey.skey[0];
 	}
+
+	length = ((union recv_frame *)precvframe)->u.hdr.len -
+		 prxattrib->hdrlen - prxattrib->iv_len;
+
+	res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+
 exit:
 	return res;
 }
-- 
2.34.1

Re: [PATCH] staging: rtl8723bs: refactor rtw_aes_decrypt() to reduce nesting

[Greg KH]

On Wed, Mar 18, 2026 at 12:51:49AM +0800, Lin YuChen wrote:
> Improve code readability by refactoring rtw_aes_decrypt() to use
> guard clauses and early exits. This reduces the maximum indentation
> level and flattens the logic flow for key assignment and decryption.
> 
> Signed-off-by: Lin YuChen <starpt.official@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_security.c | 91 ++++++++++---------
>  1 file changed, 46 insertions(+), 45 deletions(-)

Please make this a patch series, moving the indentation in by one
"level" at a time, otherwise this is impossible to review to verify that
you have not changed the logic incorrectly here.

Would you want to review this as-is?  :)

thanks,

greg k-h

[PATCH v2 0/2] staging: rtl8723bs: refactor rtw_aes_decrypt()

[Lin YuChen]

This series refactors rtw_aes_decrypt() to reduce nesting levels by
using guard clauses. As requested by Greg, the refactoring is split
into two steps to make the logic changes easier to verify.

Patch 1: Introduce guard clause for AES encryption check.
Patch 2: Introduce guard clause for stainfo check.

Lin YuChen (2):
  staging: rtl8723bs: use guard clause for AES check
  staging: rtl8723bs: use guard clause for stainfo check

 drivers/staging/rtl8723bs/core/rtw_security.c | 91 +++++++++----------
 1 file changed, 45 insertions(+), 46 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] staging: rtl8723bs: use guard clause for AES check

[Lin YuChen]

Refactor the AES encryption check by using a guard clause to
reduce the indentation level of the subsequent logic.

Signed-off-by: Lin YuChen <starpt.official@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 107 +++++++++---------
 1 file changed, 55 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index b489babe7432..971d913ec6b3 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -1213,68 +1213,71 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
 	pframe = (unsigned char *)((union recv_frame *)precvframe)->u.hdr.rx_data;
 	/* 4 start to encrypt each fragment */
-	if (prxattrib->encrypt == _AES_) {
-		stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
-		if (stainfo) {
-			if (is_multicast_ether_addr(prxattrib->ra)) {
-				static unsigned long start;
-				static u32 no_gkey_bc_cnt;
-				static u32 no_gkey_mc_cnt;
-
-				if (!psecuritypriv->binstallGrpkey) {
-					res = _FAIL;
+	if (prxattrib->encrypt != _AES_)
+		goto exit;
+	stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
+	if (stainfo) {
+		if (is_multicast_ether_addr(prxattrib->ra)) {
+			static unsigned long start;
+			static u32 no_gkey_bc_cnt;
+			static u32 no_gkey_mc_cnt;
+
+			if (!psecuritypriv->binstallGrpkey) {
+				res = _FAIL;
 
-					if (start == 0)
-						start = jiffies;
+				if (start == 0)
+					start = jiffies;
 
-					if (is_broadcast_mac_addr(prxattrib->ra))
-						no_gkey_bc_cnt++;
-					else
-						no_gkey_mc_cnt++;
-
-					if (jiffies_to_msecs(jiffies - start) > 1000) {
-						if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-							netdev_dbg(padapter->pnetdev,
-								   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-								   FUNC_ADPT_ARG(padapter),
-								   no_gkey_bc_cnt,
-								   no_gkey_mc_cnt);
-						}
-						start = jiffies;
-						no_gkey_bc_cnt = 0;
-						no_gkey_mc_cnt = 0;
+				if (is_broadcast_mac_addr(prxattrib->ra))
+					no_gkey_bc_cnt++;
+				else
+					no_gkey_mc_cnt++;
+
+				if (jiffies_to_msecs(jiffies - start) > 1000) {
+					if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+						netdev_dbg(padapter->pnetdev,
+							   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+							   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
+							   no_gkey_mc_cnt);
 					}
-
-					goto exit;
+					start = jiffies;
+					no_gkey_bc_cnt = 0;
+					no_gkey_mc_cnt = 0;
 				}
 
-				if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-					netdev_dbg(padapter->pnetdev,
-						   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-						   FUNC_ADPT_ARG(padapter),
-						   no_gkey_bc_cnt,
-						   no_gkey_mc_cnt);
-				}
-				start = 0;
-				no_gkey_bc_cnt = 0;
-				no_gkey_mc_cnt = 0;
+				goto exit;
+			}
 
-				prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
-				if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
-					res = _FAIL;
-					goto exit;
-				}
-			} else {
-				prwskey = &stainfo->dot118021x_UncstKey.skey[0];
+			if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+				netdev_dbg(padapter->pnetdev,
+					   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+					   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
+					   no_gkey_mc_cnt);
+			}
+			start = 0;
+			no_gkey_bc_cnt = 0;
+			no_gkey_mc_cnt = 0;
+
+			prwskey =
+				psecuritypriv
+					->dot118021XGrpKey[prxattrib->key_index]
+					.skey;
+			if (psecuritypriv->dot118021XGrpKeyid !=
+			    prxattrib->key_index) {
+				res = _FAIL;
+				goto exit;
 			}
+		} else {
+			prwskey = &stainfo->dot118021x_UncstKey.skey[0];
+		}
 
-			length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
+		length = ((union recv_frame *)precvframe)->u.hdr.len -
+			 prxattrib->hdrlen - prxattrib->iv_len;
 
-			res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+		res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
 
-		} else {
-			res = _FAIL;
-		}
+	} else {
+		res = _FAIL;
 	}
 exit:
 	return res;
-- 
2.34.1

Re: [PATCH v2 1/2] staging: rtl8723bs: use guard clause for AES check

[Dan Carpenter]

On Thu, Mar 19, 2026 at 05:51:23AM +0800, Lin YuChen wrote:
> Refactor the AES encryption check by using a guard clause to
> reduce the indentation level of the subsequent logic.
> 
> Signed-off-by: Lin YuChen <starpt.official@gmail.com>
> ---

Let's do it a little bit differently.  See the other email
I just sent to v1.

regards,
dan carpenter

[PATCH v3 0/2] staging: rtl8723bs: refactor rtw_aes_decrypt()

[Lin YuChen]

This series refactors rtw_aes_decrypt() to improve code readability by
reducing nesting levels through the use of guard clauses.

Changes in v3:
 - Follow Dan Carpenter's suggestion to use direct returns (e.g., return
   _SUCCESS or _FAIL) instead of "goto exit" for the newly introduced
   guard clauses.
 - Revert unrelated line-breaking and formatting changes to ensure the
   patch remains focused strictly on the refactoring of indentation
   levels, as advised by Dan.
 - Keep the two-step patch series structure requested by Greg KH.

Changes in v2:
 - Split the refactor into a two-patch series to make logic verification
   easier, as requested by Greg KH.

Lin YuChen (2):
  staging: rtl8723bs: use guard clause for AES check
  staging: rtl8723bs: use guard clause for stainfo check

 drivers/staging/rtl8723bs/core/rtw_security.c | 86 +++++++++----------
 1 file changed, 42 insertions(+), 44 deletions(-)

-- 
2.34.1

[PATCH v3 1/2] staging: rtl8723bs: use guard clause for AES check

[Lin YuChen]

Refactor the AES encryption check by using a guard clause to
reduce the indentation level of the subsequent logic.

Signed-off-by: Lin YuChen <starpt.official@gmail.com>
---
v3: Use "return _SUCCESS" instead of "goto exit".
v2: This is a new patch split from the original v1.

 drivers/staging/rtl8723bs/core/rtw_security.c | 104 +++++++++---------
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index b489babe7432..3c89e911ac2d 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -1213,68 +1213,68 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 
 	pframe = (unsigned char *)((union recv_frame *)precvframe)->u.hdr.rx_data;
 	/* 4 start to encrypt each fragment */
-	if (prxattrib->encrypt == _AES_) {
-		stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
-		if (stainfo) {
-			if (is_multicast_ether_addr(prxattrib->ra)) {
-				static unsigned long start;
-				static u32 no_gkey_bc_cnt;
-				static u32 no_gkey_mc_cnt;
-
-				if (!psecuritypriv->binstallGrpkey) {
-					res = _FAIL;
-
-					if (start == 0)
-						start = jiffies;
+	if (prxattrib->encrypt != _AES_)
+		return _SUCCESS;
+	stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
+	if (stainfo) {
+		if (is_multicast_ether_addr(prxattrib->ra)) {
+			static unsigned long start;
+			static u32 no_gkey_bc_cnt;
+			static u32 no_gkey_mc_cnt;
+
+			if (!psecuritypriv->binstallGrpkey) {
+				res = _FAIL;
 
-					if (is_broadcast_mac_addr(prxattrib->ra))
-						no_gkey_bc_cnt++;
-					else
-						no_gkey_mc_cnt++;
+				if (start == 0)
+					start = jiffies;
 
-					if (jiffies_to_msecs(jiffies - start) > 1000) {
-						if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-							netdev_dbg(padapter->pnetdev,
-								   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-								   FUNC_ADPT_ARG(padapter),
-								   no_gkey_bc_cnt,
-								   no_gkey_mc_cnt);
-						}
-						start = jiffies;
-						no_gkey_bc_cnt = 0;
-						no_gkey_mc_cnt = 0;
+				if (is_broadcast_mac_addr(prxattrib->ra))
+					no_gkey_bc_cnt++;
+				else
+					no_gkey_mc_cnt++;
+
+				if (jiffies_to_msecs(jiffies - start) > 1000) {
+					if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+						netdev_dbg(padapter->pnetdev,
+							   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+							   FUNC_ADPT_ARG(padapter),
+							   no_gkey_bc_cnt,
+							   no_gkey_mc_cnt);
 					}
-
-					goto exit;
+					start = jiffies;
+					no_gkey_bc_cnt = 0;
+					no_gkey_mc_cnt = 0;
 				}
 
-				if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-					netdev_dbg(padapter->pnetdev,
-						   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-						   FUNC_ADPT_ARG(padapter),
-						   no_gkey_bc_cnt,
-						   no_gkey_mc_cnt);
-				}
-				start = 0;
-				no_gkey_bc_cnt = 0;
-				no_gkey_mc_cnt = 0;
-
-				prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
-				if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
-					res = _FAIL;
-					goto exit;
-				}
-			} else {
-				prwskey = &stainfo->dot118021x_UncstKey.skey[0];
+				goto exit;
 			}
 
-			length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
-
-			res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+			if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+				netdev_dbg(padapter->pnetdev,
+					   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+					   FUNC_ADPT_ARG(padapter),
+					   no_gkey_bc_cnt,
+					   no_gkey_mc_cnt);
+			}
+			start = 0;
+			no_gkey_bc_cnt = 0;
+			no_gkey_mc_cnt = 0;
 
+			prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
+			if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
+				res = _FAIL;
+				goto exit;
+			}
 		} else {
-			res = _FAIL;
+			prwskey = &stainfo->dot118021x_UncstKey.skey[0];
 		}
+
+		length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
+
+		res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+
+	} else {
+		res = _FAIL;
 	}
 exit:
 	return res;
-- 
2.34.1

[PATCH v3 2/2] staging: rtl8723bs: use guard clause for stainfo check

[Lin YuChen]

Continue the refactor of rtw_aes_decrypt() by introducing a guard
clause for the stainfo check. This allows the subsequent multicast
and unicast decryption logic to be moved one indentation level to
the left, further improving code readability.

Signed-off-by: Lin YuChen <starpt.official@gmail.com>
---
v3: Use "return _FAIL" instead of "res = _FAIL; goto exit" for the stainfo
    guard clause.
v3: Revert unrelated formatting and line-breaking changes (e.g., in
    netdev_dbg calls) to keep the diff focused only on indentation
    changes, as suggested by Dan Carpenter.
v2: Split the refactoring into a separate patch to make it easier to
    verify logic changes.

 drivers/staging/rtl8723bs/core/rtw_security.c | 96 +++++++++----------
 1 file changed, 47 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index 3c89e911ac2d..cf910c268bb3 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -1216,66 +1216,64 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 	if (prxattrib->encrypt != _AES_)
 		return _SUCCESS;
 	stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
-	if (stainfo) {
-		if (is_multicast_ether_addr(prxattrib->ra)) {
-			static unsigned long start;
-			static u32 no_gkey_bc_cnt;
-			static u32 no_gkey_mc_cnt;
-
-			if (!psecuritypriv->binstallGrpkey) {
-				res = _FAIL;
+	if (stainfo)
+		return _FAIL;
+	if (is_multicast_ether_addr(prxattrib->ra)) {
+		static unsigned long start;
+		static u32 no_gkey_bc_cnt;
+		static u32 no_gkey_mc_cnt;
 
-				if (start == 0)
-					start = jiffies;
+		if (!psecuritypriv->binstallGrpkey) {
+			res = _FAIL;
 
-				if (is_broadcast_mac_addr(prxattrib->ra))
-					no_gkey_bc_cnt++;
-				else
-					no_gkey_mc_cnt++;
-
-				if (jiffies_to_msecs(jiffies - start) > 1000) {
-					if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-						netdev_dbg(padapter->pnetdev,
-							   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-							   FUNC_ADPT_ARG(padapter),
-							   no_gkey_bc_cnt,
-							   no_gkey_mc_cnt);
-					}
-					start = jiffies;
-					no_gkey_bc_cnt = 0;
-					no_gkey_mc_cnt = 0;
-				}
+			if (start == 0)
+				start = jiffies;
 
-				goto exit;
-			}
+			if (is_broadcast_mac_addr(prxattrib->ra))
+				no_gkey_bc_cnt++;
+			else
+				no_gkey_mc_cnt++;
 
-			if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-				netdev_dbg(padapter->pnetdev,
-					   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-					   FUNC_ADPT_ARG(padapter),
-					   no_gkey_bc_cnt,
-					   no_gkey_mc_cnt);
+			if (jiffies_to_msecs(jiffies - start) > 1000) {
+				if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+					netdev_dbg(padapter->pnetdev,
+						   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+						   FUNC_ADPT_ARG(padapter),
+						   no_gkey_bc_cnt,
+						   no_gkey_mc_cnt);
+				}
+				start = jiffies;
+				no_gkey_bc_cnt = 0;
+				no_gkey_mc_cnt = 0;
 			}
-			start = 0;
-			no_gkey_bc_cnt = 0;
-			no_gkey_mc_cnt = 0;
 
-			prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
-			if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
-				res = _FAIL;
-				goto exit;
-			}
-		} else {
-			prwskey = &stainfo->dot118021x_UncstKey.skey[0];
+			goto exit;
 		}
 
-		length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
-
-		res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+		if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+			netdev_dbg(padapter->pnetdev,
+				   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+				   FUNC_ADPT_ARG(padapter),
+				   no_gkey_bc_cnt,
+				   no_gkey_mc_cnt);
+		}
+		start = 0;
+		no_gkey_bc_cnt = 0;
+		no_gkey_mc_cnt = 0;
 
+		prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
+		if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
+			res = _FAIL;
+			goto exit;
+		}
 	} else {
-		res = _FAIL;
+		prwskey = &stainfo->dot118021x_UncstKey.skey[0];
 	}
+
+	length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
+
+	res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+
 exit:
 	return res;
 }
-- 
2.34.1

Re: [PATCH v3 0/2] staging: rtl8723bs: refactor rtw_aes_decrypt()

[Dan Carpenter]

On Thu, Mar 19, 2026 at 08:07:35PM +0800, Lin YuChen wrote:
> This series refactors rtw_aes_decrypt() to improve code readability by
> reducing nesting levels through the use of guard clauses.
> 
> Changes in v3:
>  - Follow Dan Carpenter's suggestion to use direct returns (e.g., return
>    _SUCCESS or _FAIL) instead of "goto exit" for the newly introduced
>    guard clauses.
>  - Revert unrelated line-breaking and formatting changes to ensure the
>    patch remains focused strictly on the refactoring of indentation
>    levels, as advised by Dan.

Thanks.

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

You don't have to do this if you don't want, but unrelated to your
patch but the last two bytes of &le_tmp64 in rtw_BIP_verify() should
be initialized.  Just set it to zero at the start of the function.
Otherwise it's an uninitialized variable.

drivers/staging/rtl8723bs/core/rtw_security.c:1308 rtw_BIP_verify() warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)

regards,
dan carpenter

Re: [PATCH v3 0/2] staging: rtl8723bs: refactor rtw_aes_decrypt()

[YuChen Lin]

On Thu, Mar 19, 2026 at 8:46 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
>
> Thanks.
>
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

Thanks for the Reviewed-by!

> You don't have to do this if you don't want, but unrelated to your
> patch but the last two bytes of &le_tmp64 in rtw_BIP_verify() should
> be initialized.  Just set it to zero at the start of the function.
> Otherwise it's an uninitialized variable.
>
> drivers/staging/rtl8723bs/core/rtw_security.c:1308 rtw_BIP_verify() warn: not copying enough bytes for '&le_tmp64' (8 vs 6 bytes)
>
> regards,
> dan carpenter

I will send a separate patch to initialize le_tmp64 in
rtw_BIP_verify() as you suggested.

Regards,
Lin YuChen

[PATCH v2 2/2] staging: rtl8723bs: use guard clause for stainfo check

[Lin YuChen]

Continue the refactor of rtw_aes_decrypt() by introducing a guard
clause for the stainfo check. This allows the subsequent multicast
and unicast decryption logic to be moved one indentation level to
the left, further improving code readability.

Signed-off-by: Lin YuChen <starpt.official@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_security.c | 100 +++++++++---------
 1 file changed, 48 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index 971d913ec6b3..0e1a89702edd 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -1216,69 +1216,65 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
 	if (prxattrib->encrypt != _AES_)
 		goto exit;
 	stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
-	if (stainfo) {
-		if (is_multicast_ether_addr(prxattrib->ra)) {
-			static unsigned long start;
-			static u32 no_gkey_bc_cnt;
-			static u32 no_gkey_mc_cnt;
+	if (!stainfo) {
+		res = _FAIL;
+		goto exit;
+	}
+	if (is_multicast_ether_addr(prxattrib->ra)) {
+		static unsigned long start;
+		static u32 no_gkey_bc_cnt;
+		static u32 no_gkey_mc_cnt;
 
-			if (!psecuritypriv->binstallGrpkey) {
-				res = _FAIL;
+		if (!psecuritypriv->binstallGrpkey) {
+			res = _FAIL;
 
-				if (start == 0)
-					start = jiffies;
+			if (start == 0)
+				start = jiffies;
 
-				if (is_broadcast_mac_addr(prxattrib->ra))
-					no_gkey_bc_cnt++;
-				else
-					no_gkey_mc_cnt++;
-
-				if (jiffies_to_msecs(jiffies - start) > 1000) {
-					if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-						netdev_dbg(padapter->pnetdev,
-							   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-							   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
-							   no_gkey_mc_cnt);
-					}
-					start = jiffies;
-					no_gkey_bc_cnt = 0;
-					no_gkey_mc_cnt = 0;
-				}
+			if (is_broadcast_mac_addr(prxattrib->ra))
+				no_gkey_bc_cnt++;
+			else
+				no_gkey_mc_cnt++;
 
-				goto exit;
+			if (jiffies_to_msecs(jiffies - start) > 1000) {
+				if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+					netdev_dbg(padapter->pnetdev,
+						   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+						   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
+						   no_gkey_mc_cnt);
+				}
+				start = jiffies;
+				no_gkey_bc_cnt = 0;
+				no_gkey_mc_cnt = 0;
 			}
 
-			if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
-				netdev_dbg(padapter->pnetdev,
-					   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
-					   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
-					   no_gkey_mc_cnt);
-			}
-			start = 0;
-			no_gkey_bc_cnt = 0;
-			no_gkey_mc_cnt = 0;
-
-			prwskey =
-				psecuritypriv
-					->dot118021XGrpKey[prxattrib->key_index]
-					.skey;
-			if (psecuritypriv->dot118021XGrpKeyid !=
-			    prxattrib->key_index) {
-				res = _FAIL;
-				goto exit;
-			}
-		} else {
-			prwskey = &stainfo->dot118021x_UncstKey.skey[0];
+			goto exit;
 		}
 
-		length = ((union recv_frame *)precvframe)->u.hdr.len -
-			 prxattrib->hdrlen - prxattrib->iv_len;
-
-		res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
+		if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
+			netdev_dbg(padapter->pnetdev,
+				   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
+				   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
+				   no_gkey_mc_cnt);
+		}
+		start = 0;
+		no_gkey_bc_cnt = 0;
+		no_gkey_mc_cnt = 0;
 
+		prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index]
+				  .skey;
+		if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
+			res = _FAIL;
+			goto exit;
+		}
 	} else {
-		res = _FAIL;
+		prwskey = &stainfo->dot118021x_UncstKey.skey[0];
 	}
+
+	length = ((union recv_frame *)precvframe)->u.hdr.len -
+		 prxattrib->hdrlen - prxattrib->iv_len;
+
+	res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
 exit:
 	return res;
 }
-- 
2.34.1

Re: [PATCH] staging: rtl8723bs: refactor rtw_aes_decrypt() to reduce nesting

[Dan Carpenter]

What often happens is that I write a review email and then decide not
to send it.  I probably would have merged your patch as-is, but since
Greg has decided not to then we as well fix this as well.

On Wed, Mar 18, 2026 at 12:51:49AM +0800, Lin YuChen wrote:
> Improve code readability by refactoring rtw_aes_decrypt() to use
> guard clauses and early exits. This reduces the maximum indentation
> level and flattens the logic flow for key assignment and decryption.
> 
> Signed-off-by: Lin YuChen <starpt.official@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_security.c | 91 ++++++++++---------
>  1 file changed, 46 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
> index b489babe7432..9229e0a1c792 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_security.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c
> @@ -1213,69 +1213,70 @@ u32 rtw_aes_decrypt(struct adapter *padapter, u8 *precvframe)
>  
>  	pframe = (unsigned char *)((union recv_frame *)precvframe)->u.hdr.rx_data;
>  	/* 4 start to encrypt each fragment */
> -	if (prxattrib->encrypt == _AES_) {
> -		stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
> -		if (stainfo) {
> -			if (is_multicast_ether_addr(prxattrib->ra)) {
> -				static unsigned long start;
> -				static u32 no_gkey_bc_cnt;
> -				static u32 no_gkey_mc_cnt;
> +	if (prxattrib->encrypt != _AES_)
> +		goto exit;

Just return directly.  When I'm reading this patch, I have to think
"Oh, what does goto exit do?"  So I scroll down to the bottom, and I
see it returns res.  So then I scroll up here and I think "what is
res?" but it's not included in the email.  Compare that to just doing
"return _SUCCESS;" where anyone can instantly see what it does without
scrolling up and down and changing to a different window.

>  
> -				if (!psecuritypriv->binstallGrpkey) {
> -					res = _FAIL;
> +	stainfo = rtw_get_stainfo(&padapter->stapriv, &prxattrib->ta[0]);
> +	if (!stainfo) {
> +		res = _FAIL;
> +		goto exit;
> +	}

Same.  Just return directly.

> +	if (is_multicast_ether_addr(prxattrib->ra)) {
> +		static unsigned long start;
> +		static u32 no_gkey_bc_cnt;
> +		static u32 no_gkey_mc_cnt;
>  
> -					if (start == 0)
> -						start = jiffies;
> +		if (!psecuritypriv->binstallGrpkey) {
> +			res = _FAIL;
>  
> -					if (is_broadcast_mac_addr(prxattrib->ra))
> -						no_gkey_bc_cnt++;
> -					else
> -						no_gkey_mc_cnt++;
> +			if (start == 0)
> +				start = jiffies;
>  
> -					if (jiffies_to_msecs(jiffies - start) > 1000) {
> -						if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
> -							netdev_dbg(padapter->pnetdev,
> -								   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
> -								   FUNC_ADPT_ARG(padapter),
> -								   no_gkey_bc_cnt,
> -								   no_gkey_mc_cnt);
> -						}
> -						start = jiffies;
> -						no_gkey_bc_cnt = 0;
> -						no_gkey_mc_cnt = 0;
> -					}
> -
> -					goto exit;

Leave this goto exit because changing it would be an unrelated change.
The line is that adding a new goto is related because you were either
going to add a return or a goto.  Changing existing code is an unrelated
change and we want to leave those lines as-is except for pulling them
in a tab.

> -				}
> +			if (is_broadcast_mac_addr(prxattrib->ra))
> +				no_gkey_bc_cnt++;
> +			else
> +				no_gkey_mc_cnt++;
>  
> +			if (jiffies_to_msecs(jiffies - start) > 1000) {
>  				if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
>  					netdev_dbg(padapter->pnetdev,
> -						   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
> -						   FUNC_ADPT_ARG(padapter),
> -						   no_gkey_bc_cnt,
> +						   FUNC_ADPT_FMT " no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
> +						   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
>  						   no_gkey_mc_cnt);
>  				}
> -				start = 0;
> +				start = jiffies;
>  				no_gkey_bc_cnt = 0;
>  				no_gkey_mc_cnt = 0;
> -
> -				prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index].skey;
> -				if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
> -					res = _FAIL;
> -					goto exit;
> -				}
> -			} else {
> -				prwskey = &stainfo->dot118021x_UncstKey.skey[0];
>  			}
>  
> -			length = ((union recv_frame *)precvframe)->u.hdr.len - prxattrib->hdrlen - prxattrib->iv_len;
> +			goto exit;
> +		}
>  
> -			res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
> +		if (no_gkey_bc_cnt || no_gkey_mc_cnt) {
> +			netdev_dbg(padapter->pnetdev,
> +				   FUNC_ADPT_FMT " gkey installed. no_gkey_bc_cnt:%u, no_gkey_mc_cnt:%u\n",
> +				   FUNC_ADPT_ARG(padapter), no_gkey_bc_cnt,
> +				   no_gkey_mc_cnt);
> +		}
> +		start = 0;
> +		no_gkey_bc_cnt = 0;
> +		no_gkey_mc_cnt = 0;
>  
> -		} else {
> +		prwskey = psecuritypriv->dot118021XGrpKey[prxattrib->key_index]
> +				  .skey;
> +		if (psecuritypriv->dot118021XGrpKeyid != prxattrib->key_index) {
>  			res = _FAIL;
> +			goto exit;
>  		}
> +	} else {
> +		prwskey = &stainfo->dot118021x_UncstKey.skey[0];
>  	}
> +
> +	length = ((union recv_frame *)precvframe)->u.hdr.len -
> +		 prxattrib->hdrlen - prxattrib->iv_len;

Checkpatch wants you to break this line up, but it wasn't broken up in
the original code so that's an unrelated change.

regards,
dan carpenter

> +
> +	res = aes_decipher(prwskey, prxattrib->hdrlen, pframe, length);
> +
>  exit:
>  	return res;
>  }
> -- 
> 2.34.1