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.1On 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
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
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
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
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.1Continue 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© 2016 - 2026 Red Hat, Inc.