[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAH1kMwTbjN4_fqG94j4Ei2SWmjSsjanwMaSJrk4M6kGOzfB9fw@mail.gmail.com>
Date: Tue, 30 Nov 2021 16:47:31 +0530
From: Vihas Mak <makvihas@...il.com>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: Larry.Finger@...inger.net, Phillip Potter <phil@...lpotter.co.uk>,
Greg KH <gregkh@...uxfoundation.org>, martin@...ser.cx,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: r8188eu: use ARRAY_SIZE() macro and fix
camelcase issues
>> Get rid of both the "arraylen" and "array" variables. They only obscure
>> what the code is doing. You may need to do additional clean up to make
>> it work without making the lines too long...
Alright.
On Tue, Nov 30, 2021 at 3:18 PM Dan Carpenter <dan.carpenter@...cle.com> wrote:
>
> On Tue, Nov 30, 2021 at 03:43:47AM +0530, Vihas Mak wrote:
> > Fix camelcase warnings from checkpatch.pl and use ARRAY_SIZE()
> > to make the code cleaner and avoid following cocci warnings:
> >
> > drivers/staging/r8188eu/hal/HalHWImg8188E_RF.c:142:51-52: WARNING: Use ARRAY_SIZE
> > drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c:174:52-53: WARNING: Use ARRAY_SIZE
> > drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c:450:52-53: WARNING: Use ARRAY_SIZE
> > drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c:655:49-50: WARNING: Use ARRAY_SIZE
> > drivers/staging/r8188eu/hal/HalHWImg8188E_MAC.c:136:50-51: WARNING: Use ARRAY_SIZE
> >
> > Signed-off-by: Vihas Mak <makvihas@...il.com>
> > ---
> > .../staging/r8188eu/hal/HalHWImg8188E_BB.c | 6 ++---
> > .../staging/r8188eu/hal/HalHWImg8188E_MAC.c | 2 +-
> > .../staging/r8188eu/hal/HalHWImg8188E_RF.c | 24 +++++++++----------
> > 3 files changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c b/drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c
> > index f6e4243e0..ce46b3651 100644
> > --- a/drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c
> > +++ b/drivers/staging/r8188eu/hal/HalHWImg8188E_BB.c
> > @@ -171,7 +171,7 @@ enum HAL_STATUS ODM_ReadAndConfig_AGC_TAB_1T_8188E(struct odm_dm_struct *dm_odm)
> > {
> > u32 hex = 0;
> > u32 i = 0;
> > - u32 arraylen = sizeof(array_agc_tab_1t_8188e) / sizeof(u32);
> > + u32 arraylen = ARRAY_SIZE(array_agc_tab_1t_8188e);
> > u32 *array = array_agc_tab_1t_8188e;
>
> Get rid of both the "arraylen" and "array" variables. They only obscure
> what the code is doing. You may need to do additional clean up to make
> it work without making the lines too long...
>
> Same for the rest.
>
> If that is too complicated then it's fine too, just leave it as-is for
> now. I know that sometimes people just want to silence the checkpatch
> warnings but I prefer to keep the warning as a marker for bad code so
> let's leave it until someone can fix this in the correct way.
>
> regards,
> dan carpenter
>
--
Thanks,
Vihas
Powered by blists - more mailing lists