Re: [PATCH 1/2] staging: rtl8723bs: Optimize variable initialization in rtl8723b_hal_init.c

Erick Karanja <[email protected]> Sun, 06 Apr 2025 07:11:56 +0300
Newsgroups dev.linux.lists.outreachy,dev.linux.lists.linux-staging,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Sat, 2025-04-05 at 17:19 +0300, Dan Carpenter 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 <[email protected]>
> > ---
> >  .../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@20130624 */
> > -	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.
Thank you on the review I believe updating the semantic patch to check
for this could be great.
> 
> > @@ -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
>