drivers/staging/rtl8723bs/core/rtw_security.c | 91 ++++++++++--------- 1 file changed, 46 insertions(+), 45 deletions(-)
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.1What 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
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
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
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.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>
---
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© 2016 - 2026 Red Hat, Inc.