On Sun, 2016-01-31 at 09:36 -0500, Jes Sorensen wrote:
> Rakhi Sharma <rakhish1994@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
> > 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
