From: Erick Karanja <karanja99erick@gmail.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: gregkh@linuxfoundation.org, outreachy@lists.linux.dev,
philipp.g.hortmann@gmail.com, linux-staging@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] staging: rtl8723bs: Optimize variable initialization in rtl8723b_hal_init.c
Date: Sun, 06 Apr 2025 07:11:56 +0300 [thread overview]
Message-ID: <b5cc9143f1ff84ac13aff6e5f56e1c14b4a181f7.camel@gmail.com> (raw)
In-Reply-To: <8ce041b2-087c-4d47-891f-28ecc0c91c76@stanley.mountain>
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 <karanja99erick@gmail.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@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
>
next prev parent reply other threads:[~2025-04-06 4:12 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-05 3:14 [PATCH 0/2] Simplify local variable initialization Erick Karanja
2025-04-05 3:14 ` [PATCH 1/2] staging: rtl8723bs: Optimize variable initialization in rtl8723b_hal_init.c Erick Karanja
2025-04-05 7:39 ` Greg KH
2025-04-05 8:39 ` Julia Lawall
2025-04-05 8:45 ` Julia Lawall
2025-04-05 10:22 ` Erick Karanja
2025-04-05 10:27 ` Julia Lawall
2025-04-05 10:35 ` Erick Karanja
2025-04-05 10:38 ` Julia Lawall
2025-04-05 14:19 ` Dan Carpenter
2025-04-05 14:28 ` Julia Lawall
2025-04-05 14:43 ` Erick Karanja
2025-04-06 4:11 ` Erick Karanja [this message]
2025-04-05 3:14 ` [PATCH 2/2] staging: rtl8723bs: Initialize local variables at declaration Erick Karanja
2025-04-05 7:37 ` Greg KH
2025-04-05 8:41 ` Julia Lawall
2025-04-05 14:21 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b5cc9143f1ff84ac13aff6e5f56e1c14b4a181f7.camel@gmail.com \
--to=karanja99erick@gmail.com \
--cc=dan.carpenter@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy@lists.linux.dev \
--cc=philipp.g.hortmann@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox