* [PATCH] staging: rtl8723bs: core: Refactor nested if-else
@ 2021-10-25 7:25 Kushal Kothari
2021-10-25 7:33 ` Greg KH
2021-10-25 9:13 ` [Outreachy kernel] " Praveen Kumar
0 siblings, 2 replies; 4+ messages in thread
From: Kushal Kothari @ 2021-10-25 7:25 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 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;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: rtl8723bs: core: Refactor nested if-else
2021-10-25 7:25 [PATCH] staging: rtl8723bs: core: Refactor nested if-else Kushal Kothari
@ 2021-10-25 7:33 ` Greg KH
[not found] ` <CALtHPQsQe06MhSvgs25TVbsjEvfU=ELbK3eFfKunRHXM1+HDxA@mail.gmail.com>
2021-10-25 9:13 ` [Outreachy kernel] " Praveen Kumar
1 sibling, 1 reply; 4+ messages in thread
From: Greg KH @ 2021-10-25 7:33 UTC (permalink / raw)
To: Kushal Kothari
Cc: fabioaiuto83, ross.schm.dev, hdegoede, marcocesati,
fmdefrancesco, linux-staging, linux-kernel, outreachy-kernel,
mike.rapoport, kushalkothari2850
On Mon, Oct 25, 2021 at 12:55:28PM +0530, 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.
Very long line, please break it up at 72 columns.
And your space around the '.' is odd :(
> checkpatch warning : Too many leading tabs - consider code refactoring
What does this mean?
>
> Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> ---
> drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 65 ++++++++-----------
> 1 file changed, 26 insertions(+), 39 deletions(-)
>
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: rtl8723bs: core: Refactor nested if-else
2021-10-25 7:25 [PATCH] staging: rtl8723bs: core: Refactor nested if-else Kushal Kothari
2021-10-25 7:33 ` Greg KH
@ 2021-10-25 9:13 ` Praveen Kumar
1 sibling, 0 replies; 4+ messages in thread
From: Praveen Kumar @ 2021-10-25 9:13 UTC (permalink / raw)
To: Kushal Kothari, gregkh, fabioaiuto83, ross.schm.dev, hdegoede,
marcocesati, fmdefrancesco, linux-staging, linux-kernel,
outreachy-kernel, mike.rapoport, kushalkothari2850
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: rtl8723bs: core: Refactor nested if-else
[not found] ` <CALtHPQsQe06MhSvgs25TVbsjEvfU=ELbK3eFfKunRHXM1+HDxA@mail.gmail.com>
@ 2021-10-25 10:02 ` Fabio M. De Francesco
0 siblings, 0 replies; 4+ messages in thread
From: Fabio M. De Francesco @ 2021-10-25 10:02 UTC (permalink / raw)
To: Greg KH, kushal kothari
Cc: fabioaiuto83, ross.schm.dev, hdegoede, marcocesati,
linux-staging, linux-kernel, outreachy-kernel, Mike Rapoport,
kushalkothari2850
On Monday, October 25, 2021 9:56:00 AM CEST kushal kothari wrote:
> >Very long line, please break it up at 72 columns.
>
> >And your space around the '.' is odd :(
>
> Yes,fixing it.
>
^^^^^^^^^^^^^^^^^^^^
You are still using spaces the wrong way :)
English, as well as all natural languages that use Latin characters, use
punctuation and a strict style that dictates how to use the spaces around it.
Please read correctly written English texts, learn and use conventions
Thanks,
Fabio
>
> >> checkpatch warning : Too many leading tabs - consider code refactoring
>
> >What does this mean?
> Asking about the warning ,correct?
> The current code is using very deep indentations (which can be avoided) and
> due to this there are many leading tabs. So the checkpatch.pl warns of
"Too
> many leading tabs - consider code refactoring"
> <http://checkpatch.pl>
>
> On Mon, Oct 25, 2021 at 1:03 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> > On Mon, Oct 25, 2021 at 12:55:28PM +0530, 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.
> >
> > Very long line, please break it up at 72 columns.
> >
> > And your space around the '.' is odd :(
> >
> > > checkpatch warning : Too many leading tabs - consider code refactoring
> >
> > What does this mean?
> >
> > >
> > > Signed-off-by: Kushal Kothari <kushalkothari285@gmail.com>
> > > ---
> > > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 65 ++++++++-----------
> > > 1 file changed, 26 insertions(+), 39 deletions(-)
> > >
> >
> > thanks,
> >
> > greg k-h
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-25 10:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 7:25 [PATCH] staging: rtl8723bs: core: Refactor nested if-else Kushal Kothari
2021-10-25 7:33 ` Greg KH
[not found] ` <CALtHPQsQe06MhSvgs25TVbsjEvfU=ELbK3eFfKunRHXM1+HDxA@mail.gmail.com>
2021-10-25 10:02 ` Fabio M. De Francesco
2021-10-25 9:13 ` [Outreachy kernel] " Praveen Kumar
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).