Joe Perches <j...@perches.com> writes:
> On Sun, 2016-01-31 at 09:36 -0500, Jes Sorensen wrote:
>> Rakhi Sharma <rakhish1...@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 <rakhish1...@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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
