* [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue.
@ 2016-01-31 13:24 Rakhi Sharma
2016-01-31 14:36 ` Jes Sorensen
0 siblings, 1 reply; 6+ messages in thread
From: Rakhi Sharma @ 2016-01-31 13:24 UTC (permalink / raw)
To: Jes.Sorensen, Larry.Finger, gregkh; +Cc: linux-wireless, devel, Rakhi Sharma
Fixed the space and brace coding style error.
ERROR: space required before that '='
ERROR: that open brace { should be on the previous line.
Signed-off-by: Rakhi Sharma <rakhish1994@gmail.com>
---
drivers/staging/rtl8723au/core/rtw_ieee80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
index 3b0d782..0c933e4 100644
--- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
@@ -65,8 +65,8 @@ static u8 WIFI_OFDMRATES[] = {
int rtw_get_bit_value_from_ieee_value23a(u8 val)
{
- unsigned char dot11_rate_table[] =
- {2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
+ unsigned char dot11_rate_table[] = {
+ 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
int i = 0;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue.
2016-01-31 13:24 [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue Rakhi Sharma
@ 2016-01-31 14:36 ` Jes Sorensen
2016-01-31 17:25 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Jes Sorensen @ 2016-01-31 14:36 UTC (permalink / raw)
To: Rakhi Sharma; +Cc: Larry.Finger, gregkh, linux-wireless, devel
Rakhi Sharma <rakhish1994@gmail.com> writes:
> Fixed the space and brace coding style error.
> ERROR: space required before that '='
> ERROR: that open brace { should be on the previous line.
>
> Signed-off-by: Rakhi Sharma <rakhish1994@gmail.com>
> ---
> drivers/staging/rtl8723au/core/rtw_ieee80211.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
> index 3b0d782..0c933e4 100644
> --- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
> +++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
> @@ -65,8 +65,8 @@ static u8 WIFI_OFDMRATES[] = {
>
> int rtw_get_bit_value_from_ieee_value23a(u8 val)
> {
> - unsigned char dot11_rate_table[] =
> - {2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
> + unsigned char dot11_rate_table[] = {
> + 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
>
> int i = 0;
This is a great example of checkpatch doing the wrong thing. What you
did here was to make the code uglier rather than better.
NACK
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue.
2016-01-31 14:36 ` Jes Sorensen
@ 2016-01-31 17:25 ` Joe Perches
2016-02-01 12:29 ` Jes Sorensen
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-01-31 17:25 UTC (permalink / raw)
To: Jes Sorensen, Rakhi Sharma; +Cc: Larry.Finger, gregkh, linux-wireless, devel
On Sun, 2016-01-31 at 09:36 -0500, Jes Sorensen wrote:
> Rakhi Sharma <rakhish1994@gmail.com> writes:
> > Fixed the space and brace coding style error.
> > ERROR: space required before that '='
> > ERROR: that open brace { should be on the previous line.
> >
> > Signed-off-by: Rakhi Sharma <rakhish1994@gmail.com>
> > ---
> > drivers/staging/rtl8723au/core/rtw_ieee80211.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
> > index 3b0d782..0c933e4 100644
> > --- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
> > +++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
> > @@ -65,8 +65,8 @@ static u8 WIFI_OFDMRATES[] = {
> >
> > int rtw_get_bit_value_from_ieee_value23a(u8 val)
> > {
> > - unsigned char dot11_rate_table[] =
> > - {2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
> > + unsigned char dot11_rate_table[] = {
> > + 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
> >
> > int i = 0;
>
> This is a great example of checkpatch doing the wrong thing. What you
> did here was to make the code uglier rather than better.
>
> NACK
Here's the original code:
int rtw_get_bit_value_from_ieee_value23a(u8 val)
{
unsigned char dot11_rate_table[]=
{2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
int i = 0;
while (dot11_rate_table[i] != 0) {
if (dot11_rate_table[i] == val)
return BIT(i);
i++;
}
return 0;
}
It has several nominal style defects:
o compares different types (unsigned char to u8)
o uses a while loop and test of a known sized array
o array isn't declared static const
o array initialization is different style from others
in the same file. "type foo[] = { bar, baz };"
o the function argument is tested on the right side
tested on left side is more common.
So this could be transformed into something like:
int rtw_get_bit_value_from_ieee_value23a(u8 val)
{
int i;
static const u8 dot11_rate_table[] = {
2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108
};
for (i = 0; i < ARRAY_SIZE(dot11_rate_table); i++) {
if (val == dot11_rate_table[i])
return BIT(i);
}
return 0;
}
Rakhi, do please strive to make the code "better" rather
than merely shut up checkpatch.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue.
2016-01-31 17:25 ` Joe Perches
@ 2016-02-01 12:29 ` Jes Sorensen
2016-02-01 14:28 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Jes Sorensen @ 2016-02-01 12:29 UTC (permalink / raw)
To: Joe Perches; +Cc: Rakhi Sharma, Larry.Finger, gregkh, linux-wireless, devel
Joe Perches <joe@perches.com> writes:
> On Sun, 2016-01-31 at 09:36 -0500, Jes Sorensen wrote:
>> Rakhi Sharma <rakhish1994@gmail.com> writes:
>> > Fixed the space and brace coding style error.
>> > ERROR: space required before that '='
>> > ERROR: that open brace { should be on the previous line.
>> >
>> > Signed-off-by: Rakhi Sharma <rakhish1994@gmail.com>
>> > ---
>> > drivers/staging/rtl8723au/core/rtw_ieee80211.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
>> > b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
>> > index 3b0d782..0c933e4 100644
>> > --- a/drivers/staging/rtl8723au/core/rtw_ieee80211.c
>> > +++ b/drivers/staging/rtl8723au/core/rtw_ieee80211.c
>> > @@ -65,8 +65,8 @@ static u8 WIFI_OFDMRATES[] = {
>> >
>> > int rtw_get_bit_value_from_ieee_value23a(u8 val)
>> > {
>> > - unsigned char dot11_rate_table[] =
>> > - {2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
>> > + unsigned char dot11_rate_table[] = {
>> > + 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
>> >
>> > int i = 0;
>>
>> This is a great example of checkpatch doing the wrong thing. What you
>> did here was to make the code uglier rather than better.
>>
>> NACK
>
> Here's the original code:
>
> int rtw_get_bit_value_from_ieee_value23a(u8 val)
> {
> unsigned char dot11_rate_table[]=
> {2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108, 0};
>
> int i = 0;
>
> while (dot11_rate_table[i] != 0) {
> if (dot11_rate_table[i] == val)
> return BIT(i);
> i++;
> }
> return 0;
> }
>
> It has several nominal style defects:
>
> o compares different types (unsigned char to u8)
> o uses a while loop and test of a known sized array
> o array isn't declared static const
> o array initialization is different style from others
> in the same file. "type foo[] = { bar, baz };"
> o the function argument is tested on the right side
> tested on left side is more common.
>
> So this could be transformed into something like:
>
> int rtw_get_bit_value_from_ieee_value23a(u8 val)
> {
> int i;
> static const u8 dot11_rate_table[] = {
> 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108
> };
and per my previous email, the above is worse than than the original.
The cleaner way to list it would be:
static const char dot11_rate_table[] =
{ 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108 };
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue.
2016-02-01 12:29 ` Jes Sorensen
@ 2016-02-01 14:28 ` Joe Perches
2016-02-01 15:02 ` Jes Sorensen
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-02-01 14:28 UTC (permalink / raw)
To: Jes Sorensen; +Cc: Rakhi Sharma, Larry.Finger, gregkh, linux-wireless, devel
On Mon, 2016-02-01 at 07:29 -0500, Jes Sorensen wrote:
> Joe Perches <joe@perches.com> writes:
[]
> > so this could be transformed into something like:
> >
> > int rtw_get_bit_value_from_ieee_value23a(u8 val)
> > {
> > int i;
> > static const u8 dot11_rate_table[] = {
> > 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108
> > };
>
> and per my previous email, the above is worse than than the original.
> The cleaner way to list it would be:
>
> static const char dot11_rate_table[] =
> { 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108 };
Either style is OK, but the style I used is slightly
more common in the kernel overall, about 2:1.
The type should ideally be u8 and not char.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue.
2016-02-01 14:28 ` Joe Perches
@ 2016-02-01 15:02 ` Jes Sorensen
0 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2016-02-01 15:02 UTC (permalink / raw)
To: Joe Perches; +Cc: Rakhi Sharma, Larry.Finger, gregkh, linux-wireless, devel
Joe Perches <joe@perches.com> writes:
> On Mon, 2016-02-01 at 07:29 -0500, Jes Sorensen wrote:
>> Joe Perches <joe@perches.com> writes:
> []
>> > so this could be transformed into something like:
>> >
>> > int rtw_get_bit_value_from_ieee_value23a(u8 val)
>> > {
>> > int i;
>> > static const u8 dot11_rate_table[] = {
>> > 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108
>> > };
>>
>> and per my previous email, the above is worse than than the original.
>> The cleaner way to list it would be:
>>
>> static const char dot11_rate_table[] =
>> { 2, 4, 11, 22, 12, 18, 24, 36, 48, 72, 96, 108 };
>
> Either style is OK, but the style I used is slightly
> more common in the kernel overall, about 2:1.
It wastes a line for now reason and having the open bracket on a line by
itself is slightly more obfuscating.
> The type should ideally be u8 and not char.
u8 or char is not a big deal for me here, I'm fine with either.
Jes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-01 15:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-31 13:24 [PATCH 3/3] Staging: rtl8723au: core: rtw_ieee80211: Fixed space and brace coding style issue Rakhi Sharma
2016-01-31 14:36 ` Jes Sorensen
2016-01-31 17:25 ` Joe Perches
2016-02-01 12:29 ` Jes Sorensen
2016-02-01 14:28 ` Joe Perches
2016-02-01 15:02 ` Jes Sorensen
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.