* [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() @ 2021-08-16 11:54 Michael Straube 2021-08-16 11:54 ` [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool Michael Straube ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Michael Straube @ 2021-08-16 11:54 UTC (permalink / raw) To: gregkh Cc: Larry.Finger, phil, martin, fmdefrancesco, linux-staging, linux-kernel, Michael Straube Refactor function rtw_is_cckrates_included(). Improves readability and slightly reduces object file size. Signed-off-by: Michael Straube <straube.linux@gmail.com> --- v1 -> v2 Refactored to more compact code as suggested by Joe Perches. drivers/staging/r8188eu/core/rtw_ieee80211.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c b/drivers/staging/r8188eu/core/rtw_ieee80211.c index 0c7231cefdda..964255a8c778 100644 --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c @@ -70,14 +70,13 @@ int rtw_get_bit_value_from_ieee_value(u8 val) uint rtw_is_cckrates_included(u8 *rate) { - u32 i = 0; + u8 r; - while (rate[i] != 0) { - if ((((rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) || - (((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22)) + while ((r = *rate++ & 0x7f)) { + if (r == 2 || r == 4 || r == 11 || r == 22) return true; - i++; } + return false; } -- 2.32.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool 2021-08-16 11:54 [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Michael Straube @ 2021-08-16 11:54 ` Michael Straube 2021-08-16 13:01 ` Fabio M. De Francesco 2021-08-16 11:54 ` [PATCH v2 3/4] staging: r8188eu: refactor rtw_is_cckratesonly_included() Michael Straube ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Michael Straube @ 2021-08-16 11:54 UTC (permalink / raw) To: gregkh Cc: Larry.Finger, phil, martin, fmdefrancesco, linux-staging, linux-kernel, Michael Straube Function rtw_is_cckrates_included() returns boolean values. Change the return type to bool to reflect this. Signed-off-by: Michael Straube <straube.linux@gmail.com> --- v1 -> v2 Rewritten to apply with v2 of patch 1/4. drivers/staging/r8188eu/core/rtw_ieee80211.c | 2 +- drivers/staging/r8188eu/include/ieee80211.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c b/drivers/staging/r8188eu/core/rtw_ieee80211.c index 964255a8c778..53556f1af425 100644 --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c @@ -68,7 +68,7 @@ int rtw_get_bit_value_from_ieee_value(u8 val) return 0; } -uint rtw_is_cckrates_included(u8 *rate) +bool rtw_is_cckrates_included(u8 *rate) { u8 r; diff --git a/drivers/staging/r8188eu/include/ieee80211.h b/drivers/staging/r8188eu/include/ieee80211.h index bc5b030e9c40..cbefd7af1d4f 100644 --- a/drivers/staging/r8188eu/include/ieee80211.h +++ b/drivers/staging/r8188eu/include/ieee80211.h @@ -1206,7 +1206,7 @@ int rtw_generate_ie(struct registry_priv *pregistrypriv); int rtw_get_bit_value_from_ieee_value(u8 val); -uint rtw_is_cckrates_included(u8 *rate); +bool rtw_is_cckrates_included(u8 *rate); uint rtw_is_cckratesonly_included(u8 *rate); -- 2.32.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool 2021-08-16 11:54 ` [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool Michael Straube @ 2021-08-16 13:01 ` Fabio M. De Francesco 0 siblings, 0 replies; 12+ messages in thread From: Fabio M. De Francesco @ 2021-08-16 13:01 UTC (permalink / raw) To: gregkh, Michael Straube Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel, Michael Straube On Monday, August 16, 2021 1:54:28 PM CEST Michael Straube wrote: > Function rtw_is_cckrates_included() returns boolean values. > Change the return type to bool to reflect this. > > Signed-off-by: Michael Straube <straube.linux@gmail.com> > --- > v1 -> v2 > Rewritten to apply with v2 of patch 1/4. Acked-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Thanks, Fabio ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] staging: r8188eu: refactor rtw_is_cckratesonly_included() 2021-08-16 11:54 [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Michael Straube 2021-08-16 11:54 ` [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool Michael Straube @ 2021-08-16 11:54 ` Michael Straube 2021-08-16 13:03 ` Fabio M. De Francesco 2021-08-16 11:54 ` [PATCH v2 4/4] staging: r8188eu: convert rtw_is_cckratesonly_included() to bool Michael Straube 2021-08-16 12:59 ` [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Fabio M. De Francesco 3 siblings, 1 reply; 12+ messages in thread From: Michael Straube @ 2021-08-16 11:54 UTC (permalink / raw) To: gregkh Cc: Larry.Finger, phil, martin, fmdefrancesco, linux-staging, linux-kernel, Michael Straube Refactor function rtw_is_cckratesonly_included(). Improves readability and slightly reduces object file size. Signed-off-by: Michael Straube <straube.linux@gmail.com> --- v1 -> v2 Refactored to more compact code as suggested by Joe Perches. drivers/staging/r8188eu/core/rtw_ieee80211.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c b/drivers/staging/r8188eu/core/rtw_ieee80211.c index 53556f1af425..b1427e70cdf7 100644 --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c @@ -82,13 +82,11 @@ bool rtw_is_cckrates_included(u8 *rate) uint rtw_is_cckratesonly_included(u8 *rate) { - u32 i = 0; + u8 r; - while (rate[i] != 0) { - if ((((rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) && - (((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f) != 22)) + while ((r = *rate++ & 0x7f)) { + if (r != 2 && r != 4 && r != 11 && r != 22) return false; - i++; } return true; -- 2.32.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/4] staging: r8188eu: refactor rtw_is_cckratesonly_included() 2021-08-16 11:54 ` [PATCH v2 3/4] staging: r8188eu: refactor rtw_is_cckratesonly_included() Michael Straube @ 2021-08-16 13:03 ` Fabio M. De Francesco 0 siblings, 0 replies; 12+ messages in thread From: Fabio M. De Francesco @ 2021-08-16 13:03 UTC (permalink / raw) To: gregkh, Michael Straube, Joe Perches Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel On Monday, August 16, 2021 1:54:29 PM CEST Michael Straube wrote: > Refactor function rtw_is_cckratesonly_included(). Improves readability > and slightly reduces object file size. > > Signed-off-by: Michael Straube <straube.linux@gmail.com> > --- > v1 -> v2 > Refactored to more compact code as suggested by Joe Perches. > > drivers/staging/r8188eu/core/rtw_ieee80211.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) Acked-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Thanks, Fabio ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] staging: r8188eu: convert rtw_is_cckratesonly_included() to bool 2021-08-16 11:54 [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Michael Straube 2021-08-16 11:54 ` [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool Michael Straube 2021-08-16 11:54 ` [PATCH v2 3/4] staging: r8188eu: refactor rtw_is_cckratesonly_included() Michael Straube @ 2021-08-16 11:54 ` Michael Straube 2021-08-16 13:04 ` Fabio M. De Francesco 2021-08-16 12:59 ` [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Fabio M. De Francesco 3 siblings, 1 reply; 12+ messages in thread From: Michael Straube @ 2021-08-16 11:54 UTC (permalink / raw) To: gregkh Cc: Larry.Finger, phil, martin, fmdefrancesco, linux-staging, linux-kernel, Michael Straube Function rtw_is_cckratesonly_included() returns boolean values. Change the return type to bool to reflect this. Signed-off-by: Michael Straube <straube.linux@gmail.com> --- v1 -> v2 Rewritten to apply with v2 of patch 3/4. drivers/staging/r8188eu/core/rtw_ieee80211.c | 2 +- drivers/staging/r8188eu/include/ieee80211.h | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c b/drivers/staging/r8188eu/core/rtw_ieee80211.c index b1427e70cdf7..a91c8d52a123 100644 --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c @@ -80,7 +80,7 @@ bool rtw_is_cckrates_included(u8 *rate) return false; } -uint rtw_is_cckratesonly_included(u8 *rate) +bool rtw_is_cckratesonly_included(u8 *rate) { u8 r; diff --git a/drivers/staging/r8188eu/include/ieee80211.h b/drivers/staging/r8188eu/include/ieee80211.h index cbefd7af1d4f..1205d13171b2 100644 --- a/drivers/staging/r8188eu/include/ieee80211.h +++ b/drivers/staging/r8188eu/include/ieee80211.h @@ -1207,8 +1207,7 @@ int rtw_generate_ie(struct registry_priv *pregistrypriv); int rtw_get_bit_value_from_ieee_value(u8 val); bool rtw_is_cckrates_included(u8 *rate); - -uint rtw_is_cckratesonly_included(u8 *rate); +bool rtw_is_cckratesonly_included(u8 *rate); int rtw_check_network_type(unsigned char *rate, int ratelen, int channel); -- 2.32.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/4] staging: r8188eu: convert rtw_is_cckratesonly_included() to bool 2021-08-16 11:54 ` [PATCH v2 4/4] staging: r8188eu: convert rtw_is_cckratesonly_included() to bool Michael Straube @ 2021-08-16 13:04 ` Fabio M. De Francesco 0 siblings, 0 replies; 12+ messages in thread From: Fabio M. De Francesco @ 2021-08-16 13:04 UTC (permalink / raw) To: gregkh, Michael Straube Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel, Michael Straube On Monday, August 16, 2021 1:54:30 PM CEST Michael Straube wrote: > Function rtw_is_cckratesonly_included() returns boolean values. > Change the return type to bool to reflect this. > > Signed-off-by: Michael Straube <straube.linux@gmail.com> > --- > v1 -> v2 > Rewritten to apply with v2 of patch 3/4. > > drivers/staging/r8188eu/core/rtw_ieee80211.c | 2 +- > drivers/staging/r8188eu/include/ieee80211.h | 3 +-- > 2 files changed, 2 insertions(+), 3 deletions(-) Acked-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Thanks, Fabio ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() 2021-08-16 11:54 [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Michael Straube ` (2 preceding siblings ...) 2021-08-16 11:54 ` [PATCH v2 4/4] staging: r8188eu: convert rtw_is_cckratesonly_included() to bool Michael Straube @ 2021-08-16 12:59 ` Fabio M. De Francesco 2021-08-16 16:15 ` Joe Perches 3 siblings, 1 reply; 12+ messages in thread From: Fabio M. De Francesco @ 2021-08-16 12:59 UTC (permalink / raw) To: gregkh, Michael Straube, Joe Perches Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel On Monday, August 16, 2021 1:54:27 PM CEST Michael Straube wrote: > Refactor function rtw_is_cckrates_included(). Improves readability > and slightly reduces object file size. > > Signed-off-by: Michael Straube <straube.linux@gmail.com> > --- > v1 -> v2 > Refactored to more compact code as suggested by Joe Perches. > > drivers/staging/r8188eu/core/rtw_ieee80211.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > Thanks for redoing the series as suggested by Joe Perches. This is a perfect case where conciseness and readability don't clash and instead the former enhances the latter. Nice work, although you chose to not take Joe's suggestion about making a helper inline function. That would have been perfect, but I think it is a minor issue. So... Acked-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Regards, Fabio P.S.: If I were you, I'd have provided a cover letter that would have helped the readers to immediately understand the purpose of the entire series. I'm not sure whether or not the above-mentioned cover is a strict requirement. > diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c > b/drivers/staging/r8188eu/core/rtw_ieee80211.c index 0c7231cefdda.. 964255a8c778 100644 > --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c > +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c > @@ -70,14 +70,13 @@ int rtw_get_bit_value_from_ieee_value(u8 val) > > uint rtw_is_cckrates_included(u8 *rate) > { > - u32 i = 0; > + u8 r; > > - while (rate[i] != 0) { > - if ((((rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) || > - (((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22)) > + while ((r = *rate++ & 0x7f)) { > + if (r == 2 || r == 4 || r == 11 || r == 22) > return true; > - i++; > } > + > return false; > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() 2021-08-16 12:59 ` [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Fabio M. De Francesco @ 2021-08-16 16:15 ` Joe Perches 0 siblings, 0 replies; 12+ messages in thread From: Joe Perches @ 2021-08-16 16:15 UTC (permalink / raw) To: Fabio M. De Francesco, gregkh, Michael Straube Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel On Mon, 2021-08-16 at 14:59 +0200, Fabio M. De Francesco wrote: > On Monday, August 16, 2021 1:54:27 PM CEST Michael Straube wrote: > > Refactor function rtw_is_cckrates_included(). Improves readability > > and slightly reduces object file size. > > > > Signed-off-by: Michael Straube <straube.linux@gmail.com> > > --- > > v1 -> v2 > > Refactored to more compact code as suggested by Joe Perches. > > > > drivers/staging/r8188eu/core/rtw_ieee80211.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > Thanks for redoing the series as suggested by Joe Perches. > This is a perfect case where conciseness and readability don't clash and > instead the former enhances the latter. Perhaps do the whole thing in one go (moving the & 0x7f into the helper avoids an early loop exit defect when the rate being indexed is 0x80) --- drivers/staging/r8188eu/core/rtw_ieee80211.c | 33 +++++++++++++++------------- drivers/staging/r8188eu/include/ieee80211.h | 5 ++--- drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 ++-- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c b/drivers/staging/r8188eu/core/rtw_ieee80211.c index ff77e686721ce..f02863caddde7 100644 --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c @@ -68,28 +68,31 @@ int rtw_get_bit_value_from_ieee_value(u8 val) return 0; } -uint rtw_is_cckrates_included(u8 *rate) +static bool rtw_is_cckrate(u8 rate) { - u32 i = 0; + rate &= 0x7f; + return rate == 2 || rate == 4 || rate == 11 || rate == 22; +} + +bool rtw_is_cckrates_included(u8 *rate) +{ + u8 r; - while (rate[i] != 0) { - if ((((rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) || - (((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22)) + while ((r = *rate++)) { + if (rtw_is_cckrate(r)) return true; - i++; } + return false; } -uint rtw_is_cckratesonly_included(u8 *rate) +bool rtw_is_cckratesonly_included(u8 *rate) { - u32 i = 0; + u8 r; - while (rate[i] != 0) { - if ((((rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) && - (((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f) != 22)) + while ((r = *rate++)) { + if (!rtw_is_cckrate(r)) return false; - i++; } return true; @@ -98,14 +101,14 @@ uint rtw_is_cckratesonly_included(u8 *rate) int rtw_check_network_type(unsigned char *rate, int ratelen, int channel) { if (channel > 14) { - if ((rtw_is_cckrates_included(rate)) == true) + if ((rtw_is_cckrates_included(rate))) return WIRELESS_INVALID; else return WIRELESS_11A; } else { /* could be pure B, pure G, or B/G */ - if ((rtw_is_cckratesonly_included(rate)) == true) + if ((rtw_is_cckratesonly_included(rate))) return WIRELESS_11B; - else if ((rtw_is_cckrates_included(rate)) == true) + else if ((rtw_is_cckrates_included(rate))) return WIRELESS_11BG; else return WIRELESS_11G; diff --git a/drivers/staging/r8188eu/include/ieee80211.h b/drivers/staging/r8188eu/include/ieee80211.h index 4dfa817175e77..890419d201903 100644 --- a/drivers/staging/r8188eu/include/ieee80211.h +++ b/drivers/staging/r8188eu/include/ieee80211.h @@ -1225,9 +1225,8 @@ int rtw_generate_ie(struct registry_priv *pregistrypriv); int rtw_get_bit_value_from_ieee_value(u8 val); -uint rtw_is_cckrates_included(u8 *rate); - -uint rtw_is_cckratesonly_included(u8 *rate); +bool rtw_is_cckrates_included(u8 *rate); +bool rtw_is_cckratesonly_included(u8 *rate); int rtw_check_network_type(unsigned char *rate, int ratelen, int channel); diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c index a94946ad11fce..37ab633acb982 100644 --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c @@ -784,12 +784,12 @@ static int rtw_wx_get_name(struct net_device *dev, prates = &pcur_bss->SupportedRates; - if (rtw_is_cckratesonly_included((u8 *)prates) == true) { + if (rtw_is_cckratesonly_included((u8 *)prates)) { if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bn"); else snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11b"); - } else if ((rtw_is_cckrates_included((u8 *)prates)) == true) { + } else if ((rtw_is_cckrates_included((u8 *)prates))) { if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bgn"); else ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() @ 2021-08-16 16:15 ` Joe Perches 0 siblings, 0 replies; 12+ messages in thread From: Joe Perches @ 2021-08-16 16:15 UTC (permalink / raw) To: Fabio M. De Francesco, gregkh, Michael Straube Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel On Mon, 2021-08-16 at 14:59 +0200, Fabio M. De Francesco wrote: > On Monday, August 16, 2021 1:54:27 PM CEST Michael Straube wrote: > > Refactor function rtw_is_cckrates_included(). Improves readability > > and slightly reduces object file size. > > > > Signed-off-by: Michael Straube <straube.linux@gmail.com> > > --- > > v1 -> v2 > > Refactored to more compact code as suggested by Joe Perches. > > > > drivers/staging/r8188eu/core/rtw_ieee80211.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > Thanks for redoing the series as suggested by Joe Perches. > This is a perfect case where conciseness and readability don't clash and > instead the former enhances the latter. Perhaps do the whole thing in one go (moving the & 0x7f into the helper avoids an early loop exit defect when the rate being indexed is 0x80) --- drivers/staging/r8188eu/core/rtw_ieee80211.c | 33 +++++++++++++++------------- drivers/staging/r8188eu/include/ieee80211.h | 5 ++--- drivers/staging/r8188eu/os_dep/ioctl_linux.c | 4 ++-- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_ieee80211.c b/drivers/staging/r8188eu/core/rtw_ieee80211.c index ff77e686721ce..f02863caddde7 100644 --- a/drivers/staging/r8188eu/core/rtw_ieee80211.c +++ b/drivers/staging/r8188eu/core/rtw_ieee80211.c @@ -68,28 +68,31 @@ int rtw_get_bit_value_from_ieee_value(u8 val) return 0; } -uint rtw_is_cckrates_included(u8 *rate) +static bool rtw_is_cckrate(u8 rate) { - u32 i = 0; + rate &= 0x7f; + return rate == 2 || rate == 4 || rate == 11 || rate == 22; +} + +bool rtw_is_cckrates_included(u8 *rate) +{ + u8 r; - while (rate[i] != 0) { - if ((((rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) || - (((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22)) + while ((r = *rate++)) { + if (rtw_is_cckrate(r)) return true; - i++; } + return false; } -uint rtw_is_cckratesonly_included(u8 *rate) +bool rtw_is_cckratesonly_included(u8 *rate) { - u32 i = 0; + u8 r; - while (rate[i] != 0) { - if ((((rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) && - (((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f) != 22)) + while ((r = *rate++)) { + if (!rtw_is_cckrate(r)) return false; - i++; } return true; @@ -98,14 +101,14 @@ uint rtw_is_cckratesonly_included(u8 *rate) int rtw_check_network_type(unsigned char *rate, int ratelen, int channel) { if (channel > 14) { - if ((rtw_is_cckrates_included(rate)) == true) + if ((rtw_is_cckrates_included(rate))) return WIRELESS_INVALID; else return WIRELESS_11A; } else { /* could be pure B, pure G, or B/G */ - if ((rtw_is_cckratesonly_included(rate)) == true) + if ((rtw_is_cckratesonly_included(rate))) return WIRELESS_11B; - else if ((rtw_is_cckrates_included(rate)) == true) + else if ((rtw_is_cckrates_included(rate))) return WIRELESS_11BG; else return WIRELESS_11G; diff --git a/drivers/staging/r8188eu/include/ieee80211.h b/drivers/staging/r8188eu/include/ieee80211.h index 4dfa817175e77..890419d201903 100644 --- a/drivers/staging/r8188eu/include/ieee80211.h +++ b/drivers/staging/r8188eu/include/ieee80211.h @@ -1225,9 +1225,8 @@ int rtw_generate_ie(struct registry_priv *pregistrypriv); int rtw_get_bit_value_from_ieee_value(u8 val); -uint rtw_is_cckrates_included(u8 *rate); - -uint rtw_is_cckratesonly_included(u8 *rate); +bool rtw_is_cckrates_included(u8 *rate); +bool rtw_is_cckratesonly_included(u8 *rate); int rtw_check_network_type(unsigned char *rate, int ratelen, int channel); diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c index a94946ad11fce..37ab633acb982 100644 --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c @@ -784,12 +784,12 @@ static int rtw_wx_get_name(struct net_device *dev, prates = &pcur_bss->SupportedRates; - if (rtw_is_cckratesonly_included((u8 *)prates) == true) { + if (rtw_is_cckratesonly_included((u8 *)prates)) { if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bn"); else snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11b"); - } else if ((rtw_is_cckrates_included((u8 *)prates)) == true) { + } else if ((rtw_is_cckrates_included((u8 *)prates))) { if (ht_cap) snprintf(wrqu->name, IFNAMSIZ, "IEEE 802.11bgn"); else ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() 2021-08-16 16:15 ` Joe Perches (?) @ 2021-08-16 16:29 ` Michael Straube -1 siblings, 0 replies; 12+ messages in thread From: Michael Straube @ 2021-08-16 16:29 UTC (permalink / raw) To: Joe Perches, Fabio M. De Francesco, gregkh Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel On 8/16/21 6:15 PM, Joe Perches wrote: > On Mon, 2021-08-16 at 14:59 +0200, Fabio M. De Francesco wrote: >> On Monday, August 16, 2021 1:54:27 PM CEST Michael Straube wrote: >>> Refactor function rtw_is_cckrates_included(). Improves readability >>> and slightly reduces object file size. >>> >>> Signed-off-by: Michael Straube <straube.linux@gmail.com> >>> --- >>> v1 -> v2 >>> Refactored to more compact code as suggested by Joe Perches. >>> >>> drivers/staging/r8188eu/core/rtw_ieee80211.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >> >> Thanks for redoing the series as suggested by Joe Perches. >> This is a perfect case where conciseness and readability don't clash and >> instead the former enhances the latter. > > Perhaps do the whole thing in one go (moving the & 0x7f into the helper > avoids an early loop exit defect when the rate being indexed is 0x80) > I will send v3 with your suggestions. The comparsions to boolean ( ... == true ) will be addressed in a separate series that cleans up comparsions to true/false in all files. Regards, Michael ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() 2021-08-16 16:15 ` Joe Perches (?) (?) @ 2021-08-16 19:36 ` Michael Straube -1 siblings, 0 replies; 12+ messages in thread From: Michael Straube @ 2021-08-16 19:36 UTC (permalink / raw) To: Joe Perches, Fabio M. De Francesco, gregkh Cc: Larry.Finger, phil, martin, linux-staging, linux-kernel On 8/16/21 6:15 PM, Joe Perches wrote: > On Mon, 2021-08-16 at 14:59 +0200, Fabio M. De Francesco wrote: >> On Monday, August 16, 2021 1:54:27 PM CEST Michael Straube wrote: >>> Refactor function rtw_is_cckrates_included(). Improves readability >>> and slightly reduces object file size. >>> >>> Signed-off-by: Michael Straube <straube.linux@gmail.com> >>> --- >>> v1 -> v2 >>> Refactored to more compact code as suggested by Joe Perches. >>> >>> drivers/staging/r8188eu/core/rtw_ieee80211.c | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >>> >> >> Thanks for redoing the series as suggested by Joe Perches. >> This is a perfect case where conciseness and readability don't clash and >> instead the former enhances the latter. > > Perhaps do the whole thing in one go (moving the & 0x7f into the helper > avoids an early loop exit defect when the rate being indexed is 0x80) > I have sent a new patch that does the refactoring in one go, so this series is obsolete now. ;) Regards, Michael ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-08-16 21:01 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-16 11:54 [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Michael Straube 2021-08-16 11:54 ` [PATCH v2 2/4] staging: r8188eu: convert rtw_is_cckrates_included() to bool Michael Straube 2021-08-16 13:01 ` Fabio M. De Francesco 2021-08-16 11:54 ` [PATCH v2 3/4] staging: r8188eu: refactor rtw_is_cckratesonly_included() Michael Straube 2021-08-16 13:03 ` Fabio M. De Francesco 2021-08-16 11:54 ` [PATCH v2 4/4] staging: r8188eu: convert rtw_is_cckratesonly_included() to bool Michael Straube 2021-08-16 13:04 ` Fabio M. De Francesco 2021-08-16 12:59 ` [PATCH v2 1/4] staging: r8188eu: refactor rtw_is_cckrates_included() Fabio M. De Francesco 2021-08-16 16:15 ` Joe Perches 2021-08-16 16:15 ` Joe Perches 2021-08-16 16:29 ` Michael Straube 2021-08-16 19:36 ` Michael Straube
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.