lkml.org 
[lkml]   [2021]   [Oct]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [Outreachy kernel] [PATCH] staging: rtl8723bs: core: Refactor nested if-else
From
On 25-10-2021 12:55, Kushal Kothari wrote:
> Refactor nested if else by combining nested if into a single if condition and removing unnecessary else conditionals which leads to removing unnecessary tabs .There is no change in logic of new code.
> checkpatch warning : Too many leading tabs - consider code refactoring
>
> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 65 ++++++++-----------
> 1 file changed, 26 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> index 0f82f5031c43..eb10b6f85426 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -1192,46 +1192,33 @@ unsigned int OnAssocReq(struct adapter *padapter, union recv_frame *precv_frame)
> p = pframe + WLAN_HDR_A3_LEN + ie_offset; ie_len = 0;
> for (;;) {
> p = rtw_get_ie(p, WLAN_EID_VENDOR_SPECIFIC, &ie_len, pkt_len - WLAN_HDR_A3_LEN - ie_offset);
> - if (p) {
> - if (!memcmp(p+2, WMM_IE, 6)) {
> -
> - pstat->flags |= WLAN_STA_WME;
> -
> - pstat->qos_option = 1;
> - pstat->qos_info = *(p+8);
> -
> - pstat->max_sp_len = (pstat->qos_info>>5)&0x3;
> -
> - if ((pstat->qos_info&0xf) != 0xf)
> - pstat->has_legacy_ac = true;
> - else
> - pstat->has_legacy_ac = false;
> -
> - if (pstat->qos_info&0xf) {
> - if (pstat->qos_info&BIT(0))
> - pstat->uapsd_vo = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_vo = 0;
> -
> - if (pstat->qos_info&BIT(1))
> - pstat->uapsd_vi = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_vi = 0;
> -
> - if (pstat->qos_info&BIT(2))
> - pstat->uapsd_bk = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_bk = 0;
> -
> - if (pstat->qos_info&BIT(3))
> - pstat->uapsd_be = BIT(0)|BIT(1);
> - else
> - pstat->uapsd_be = 0;
> -
> - }
> -
> - break;
> + if (p && !memcmp(p+2, WMM_IE, 6)) {
> + pstat->flags |= WLAN_STA_WME;
> + pstat->qos_option = 1;
> + pstat->qos_info = *(p+8);
> + pstat->max_sp_len = (pstat->qos_info>>5)&0x3;
> + pstat->has_legacy_ac = false;
> + if ((pstat->qos_info&0xf) != 0xf)
> + pstat->has_legacy_ac = true;
> +
> + pstat->uapsd_vo = 0;
> + if (pstat->qos_info&0xf) {
> + if (pstat->qos_info&BIT(0))
> + pstat->uapsd_vo = BIT(0)|BIT(1);
> +
> + pstat->uapsd_vi = 0;
> + if (pstat->qos_info&BIT(1))
> + pstat->uapsd_vi = BIT(0)|BIT(1);
> +
> + pstat->uapsd_bk = 0;
> + if (pstat->qos_info&BIT(2))
> + pstat->uapsd_bk = BIT(0)|BIT(1);
> +
> + pstat->uapsd_be = 0;
> + if (pstat->qos_info&BIT(3))
> + pstat->uapsd_be = BIT(0)|BIT(1);
> }
> + break;
> } else {
> break;
> }

there is a bug here, if *p* is not null, and *memcmp* failed; then we fall in else part and break, and will miss the next entry in *p* using below statement

p = p + ie_len + 2;


>

Regards,

~Praveen.

\
 
 \ /
  Last update: 2021-10-25 11:14    [W:0.077 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site