[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <92f451551548518fe2bc6ebbdbc84efb8cf5ca32.camel@gmail.com>
Date: Sat, 05 Apr 2025 17:43:35 +0300
From: Erick Karanja <karanja99erick@...il.com>
To: Julia Lawall <Julia.Lawall@...ia.fr>, Dan Carpenter
<dan.carpenter@...aro.org>
Cc: gregkh@...uxfoundation.org, outreachy@...ts.linux.dev,
philipp.g.hortmann@...il.com, linux-staging@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] staging: rtl8723bs: Optimize variable
initialization in rtl8723b_hal_init.c
On Sat, 2025-04-05 at 10:28 -0400, Julia Lawall wrote:
>
> > On 5 Apr 2025, at 10:19, Dan Carpenter <dan.carpenter@...aro.org>
> > wrote:
> >
> > On Sat, Apr 05, 2025 at 06:14:48AM +0300, Erick Karanja wrote:
> > > Optimize variable initialization by integrating the
> > > initialization
> > > directly into the variable declaration in cases where the
> > > initialization
> > > is simple and doesn't depend on other variables or complex
> > > expressions.
> > > This makes the code more concise and readable.
> > >
> > > Signed-off-by: Erick Karanja <karanja99erick@...il.com>
> > > ---
> > > .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 155 +++++--------
> > > -----
> > > 1 file changed, 41 insertions(+), 114 deletions(-)
> > >
> > > diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > > b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > > index e15ec6452fd0..1e980b291e90 100644
> > > --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > > +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > > @@ -152,13 +152,12 @@ static int _WriteFW(struct adapter
> > > *padapter, void *buffer, u32 size)
> > > void _8051Reset8723(struct adapter *padapter)
> > > {
> > > u8 cpu_rst;
> > > - u8 io_rst;
> > > + u8 io_rst = rtw_read8(padapter, REG_RSV_CTRL + 1);
> > >
> > >
> > > /* Reset 8051(WLMCU) IO wrapper */
> > > /* 0x1c[8] = 0 */
> > > /* Suggested by Isaac@SD1 and Gimmy@SD1, coding by
> > > Lucas@...30624 */
> > > - io_rst = rtw_read8(padapter, REG_RSV_CTRL+1);
> > > io_rst &= ~BIT(0);
> > > rtw_write8(padapter, REG_RSV_CTRL+1, io_rst);
> >
> > I hate this. It's a bad idea to put "code" in the declaration
> > block.
>
> Erick, you can look around in the output of the semantic patch and
> see if all of the ones with function calls are undesirable. If that’s
> the case you can post to the outreachy mailing list a revised
> semantic patch that doesn’t report on that case.
Thanks Julia I will look at it.
>
> Julia
>
> > > @@ -501,8 +499,7 @@ void Hal_GetEfuseDefinition(
> > > switch (type) {
> > > case TYPE_EFUSE_MAX_SECTION:
> > > {
> > > - u8 *pMax_section;
> > > - pMax_section = pOut;
> > > + u8 *pMax_section = pOut;
> >
> > This is fine because "pOut" is a variable. It doesn't have side
> > effects
> > and it's not "code" in that sense.
> >
> > regards,
> > dan carpenter
> >
> >
>
Powered by blists - more mailing lists