Hi, On 10/26/21 17:12, kushal kothari wrote: > Yes! > Also initializing variable pstat->has_legacy_ac = false; //false > which is previously set to true before the loop so it can be removed too. Note please do not make too many changes in a single patch. It is best to do refactoring like this in small steps making only 1 change at a time, that makes things a lot easier to review / to verify that there are no functional changes. Regards, Hans > > On Tue, Oct 26, 2021 at 7:49 PM Mike Rapoport <mike.rapoport@xxxxxxxxx <mailto:mike.rapoport@xxxxxxxxx>> wrote: > > On Tue, Oct 26, 2021 at 07:12:53PM +0530, Kushal Kothari wrote: > > Refactor nested if-else to avoid deep indentations. There is no change > > in the logic of the new code, however, now it is simple because it gets > > rid of five unnecessary else conditionals and it combines nested if into > > single if-else-if. This refactor also leads to fix warning detected by > > checkpatch.pl <http://checkpatch.pl>: > > WARNING: Too many leading tabs - consider code refactoring > > > > Signed-off-by: Kushal Kothari <kushalkothari285@xxxxxxxxx <mailto:kushalkothari285@xxxxxxxxx>> > > --- > > > > Changes in v2: Fix the bug of not handling properly the else logic > > when p is not null in else-if. Also, reword the subject line and break > > it up at 72 columns. > > > > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 69 ++++++++----------- > > 1 file changed, 29 insertions(+), 40 deletions(-) > > > > diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > > index 0f82f5031c43..267d853b1514 100644 > > --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > > +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > > @@ -1192,50 +1192,39 @@ 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)) { > > + p = p + ie_len + 2; > > + } else 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; > > + > > + if (pstat->qos_info&0xf) { > > + pstat->uapsd_vo = 0; > > This variable and other variables below are set to 0 just before the loop, > so the initialization here can be removed. > > > + 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; > > } > > - p = p + ie_len + 2; > > } > > } > > > > -- > > 2.25.1 > > > > -- > Sincerely yours, > Mike. >
