* [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else
@ 2021-10-26 13:42 Kushal Kothari
2021-10-26 13:54 ` [Outreachy kernel] " Praveen Kumar
2021-10-26 14:19 ` Mike Rapoport
0 siblings, 2 replies; 5+ messages in thread
From: Kushal Kothari @ 2021-10-26 13:42 UTC (permalink / raw)
To: gregkh, fabioaiuto83, ross.schm.dev, hdegoede, marcocesati,
fmdefrancesco, linux-staging, linux-kernel, outreachy-kernel,
mike.rapoport, kushalkothari2850
Cc: Kushal Kothari
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:
WARNING: Too many leading tabs - consider code refactoring
Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
---
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;
+ 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else
2021-10-26 13:42 [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else Kushal Kothari
@ 2021-10-26 13:54 ` Praveen Kumar
[not found] ` <CALtHPQsh=0LyM3K9dn8d0X+n4gbx2TApF6k_WhpypHXGj6Hj-w@mail.gmail.com>
2021-10-26 14:19 ` Mike Rapoport
1 sibling, 1 reply; 5+ messages in thread
From: Praveen Kumar @ 2021-10-26 13:54 UTC (permalink / raw)
To: Kushal Kothari, gregkh, fabioaiuto83, ross.schm.dev, hdegoede,
marcocesati, fmdefrancesco, linux-staging, linux-kernel,
outreachy-kernel, mike.rapoport, kushalkothari2850
On 26-10-2021 19:12, 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:
> WARNING: Too many leading tabs - consider code refactoring
>
> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> ---
>
> 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)) {
I would think of something,
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)) {
p = p + ie_len + 2;
continue;
}
pstat->flags |= WLAN_STA_WME;
...
}
/* Here we will reach only if p is NULL or its required entry */
break;
}
Can you please check if this works well. Thanks.
Also, wanted to check how are you testing these changes ?
> + 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;
> + 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;
> }
> }
>
>
Regards,
~Praveen.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else
2021-10-26 13:42 [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else Kushal Kothari
2021-10-26 13:54 ` [Outreachy kernel] " Praveen Kumar
@ 2021-10-26 14:19 ` Mike Rapoport
[not found] ` <CALtHPQuKDa23Ui4wBVed=7FOkf6u+-MjL1VU-4CD6hXSjmsq6g@mail.gmail.com>
1 sibling, 1 reply; 5+ messages in thread
From: Mike Rapoport @ 2021-10-26 14:19 UTC (permalink / raw)
To: Kushal Kothari
Cc: gregkh, fabioaiuto83, ross.schm.dev, hdegoede, marcocesati,
fmdefrancesco, linux-staging, linux-kernel, outreachy-kernel,
kushalkothari2850
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:
> WARNING: Too many leading tabs - consider code refactoring
>
> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> ---
>
> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else
[not found] ` <CALtHPQuKDa23Ui4wBVed=7FOkf6u+-MjL1VU-4CD6hXSjmsq6g@mail.gmail.com>
@ 2021-10-26 15:15 ` Hans de Goede
0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-10-26 15:15 UTC (permalink / raw)
To: kushal kothari, Mike Rapoport
Cc: Greg Kroah-Hartman, fabioaiuto83, ross.schm.dev, marcocesati,
Fabio M. De Francesco, linux-staging, linux-kernel,
outreachy-kernel, kushalkothari2850
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@gmail.com <mailto:mike.rapoport@gmail.com>> 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@gmail.com <mailto:kushalkothari285@gmail.com>>
> > ---
> >
> > 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.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else
[not found] ` <CALtHPQsh=0LyM3K9dn8d0X+n4gbx2TApF6k_WhpypHXGj6Hj-w@mail.gmail.com>
@ 2021-10-26 17:36 ` Praveen Kumar
0 siblings, 0 replies; 5+ messages in thread
From: Praveen Kumar @ 2021-10-26 17:36 UTC (permalink / raw)
To: kushal kothari
Cc: Greg Kroah-Hartman, fabioaiuto83, ross.schm.dev, hdegoede,
marcocesati, Fabio M. De Francesco, linux-staging, linux-kernel,
outreachy-kernel, Mike Rapoport, kushalkothari2850
On 26-10-2021 20:00, kushal kothari wrote:
>> Can you please check if this works well. Thanks.
>
> checkpatch.pl again warns of
> "WARNING: Too many leading tabs - consider code refactoring"
> when adding another if loop on top .
>
Thanks for checking. The whole intention was to reduce unnecessary memcmp.
These operations are heavy and performing same comparison twice, IMHO needs to be thought well.
But, I'll wait for others to comment on this. Thanks.
Regards,
~Praveen.
> On Tue, Oct 26, 2021 at 7:24 PM Praveen Kumar <kpraveen.lkml@gmail.com>
> wrote:
>
>> On 26-10-2021 19:12, 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:
>>> WARNING: Too many leading tabs - consider code refactoring
>>>
>>> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
>>> ---
>>>
>>> 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)) {
>>
>> I would think of something,
>>
>> 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)) {
>> p = p + ie_len + 2;
>> continue;
>> }
>> pstat->flags |= WLAN_STA_WME;
>> ...
>> }
>> /* Here we will reach only if p is NULL or its required entry */
>> break;
>> }
>>
>> Can you please check if this works well. Thanks.
>> Also, wanted to check how are you testing these changes ?
>>
>>> + 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;
>>> + 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;
>>> }
>>> }
>>>
>>>
>>
>> Regards,
>>
>> ~Praveen.
>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-26 17:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 13:42 [PATCH v2] staging: rtl8723bs: core: Refactor nested if-else Kushal Kothari
2021-10-26 13:54 ` [Outreachy kernel] " Praveen Kumar
[not found] ` <CALtHPQsh=0LyM3K9dn8d0X+n4gbx2TApF6k_WhpypHXGj6Hj-w@mail.gmail.com>
2021-10-26 17:36 ` Praveen Kumar
2021-10-26 14:19 ` Mike Rapoport
[not found] ` <CALtHPQuKDa23Ui4wBVed=7FOkf6u+-MjL1VU-4CD6hXSjmsq6g@mail.gmail.com>
2021-10-26 15:15 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).