Re: [PATCH v2 1/2] staging: rtl8723bs: Improve code readability

On Wed, 2025-04-09 at 07:04 -0400, Julia Lawall wrote:
>=20
>=20
> On Wed, 9 Apr 2025, Erick Karanja wrote:
>=20
> > Make the code more readable by moving trivial
> > initializations up with the declarations instead
> > of wasting a line on that.
> >=20
> > Signed-off-by: Erick Karanja <[email protected]>
> > ---
> > =C2=A0.../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 84 ++++++--------=
-
> > ----
> > =C2=A01 file changed, 25 insertions(+), 59 deletions(-)
> >=20
> > diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > index e15ec6452fd0..2cf2c66140f1 100644
> > --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> > @@ -501,8 +501,7 @@ void Hal_GetEfuseDefinition(
> > =C2=A0	switch (type) {
> > =C2=A0	case TYPE_EFUSE_MAX_SECTION:
> > =C2=A0		{
> > -			u8 *pMax_section;
> > -			pMax_section =3D pOut;
> > +			u8 *pMax_section =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pMax_section =3D
> > EFUSE_MAX_SECTION_8723B;
> > @@ -513,8 +512,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	case TYPE_EFUSE_REAL_CONTENT_LEN:
> > =C2=A0		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp =3D pOut;
> > +			u16 *pu2Tmp =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pu2Tmp =3D
> > EFUSE_REAL_CONTENT_LEN_8723B;
> > @@ -525,8 +523,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	case TYPE_AVAILABLE_EFUSE_BYTES_BANK:
> > =C2=A0		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp =3D pOut;
> > +			u16 *pu2Tmp =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pu2Tmp =3D
> > (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> > @@ -537,8 +534,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	case TYPE_AVAILABLE_EFUSE_BYTES_TOTAL:
> > =C2=A0		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp =3D pOut;
> > +			u16 *pu2Tmp =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pu2Tmp =3D
> > (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> > @@ -549,8 +545,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	case TYPE_EFUSE_MAP_LEN:
> > =C2=A0		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp =3D pOut;
> > +			u16 *pu2Tmp =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pu2Tmp =3D EFUSE_MAX_MAP_LEN;
> > @@ -561,8 +556,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	case TYPE_EFUSE_PROTECT_BYTES_BANK:
> > =C2=A0		{
> > -			u8 *pu1Tmp;
> > -			pu1Tmp =3D pOut;
> > +			u8 *pu1Tmp =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pu1Tmp =3D EFUSE_OOB_PROTECT_BYTES;
> > @@ -573,8 +567,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	case TYPE_EFUSE_CONTENT_LEN_BANK:
> > =C2=A0		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp =3D pOut;
> > +			u16 *pu2Tmp =3D pOut;
> >=20
> > =C2=A0			if (efuseType =3D=3D EFUSE_WIFI)
> > =C2=A0				*pu2Tmp =3D
> > EFUSE_REAL_CONTENT_LEN_8723B;
> > @@ -585,8 +578,7 @@ void Hal_GetEfuseDefinition(
> >=20
> > =C2=A0	default:
> > =C2=A0		{
> > -			u8 *pu1Tmp;
> > -			pu1Tmp =3D pOut;
> > +			u8 *pu1Tmp =3D pOut;
> > =C2=A0			*pu1Tmp =3D 0;
> > =C2=A0		}
> > =C2=A0		break;
> > @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
> > =C2=A0		}
> >=20
> > =C2=A0		if (offset < EFUSE_MAX_SECTION_8723B) {
> > -			u16 addr;
> > +			u16 addr =3D offset * PGPKT_DATA_SIZE;
> > =C2=A0			/*=C2=A0 Get word enable value from PG header
> > */
> >=20
> > -			addr =3D offset * PGPKT_DATA_SIZE;
>=20
> I'm not sure about this one, due to the comment.
Hello Julia, I have observed in some scenarios where the semantic patch
deletes the comments that exist between the declaration and
initialization which I think it's not okay and in this scenario where
the comments are left dangling such as this. How do I go about this
instances.
Thank you.=20
>=20
> julia
>=20
> > =C2=A0			for (i =3D 0; i < EFUSE_MAX_WORD_UNIT; i++)
> > {
> > =C2=A0				/*=C2=A0 Check word enable condition in
> > the section */
> > =C2=A0				if (!(wden & (0x01<<i))) {
> > @@ -835,9 +826,8 @@ static void hal_ReadEFuse_BT(
> > =C2=A0			}
> >=20
> > =C2=A0			if (offset < EFUSE_BT_MAX_SECTION) {
> > -				u16 addr;
> > +				u16 addr =3D offset *
> > PGPKT_DATA_SIZE;
> >=20
> > -				addr =3D offset * PGPKT_DATA_SIZE;
> > =C2=A0				for (i =3D 0; i <
> > EFUSE_MAX_WORD_UNIT; i++) {
> > =C2=A0					/*=C2=A0 Check word enable
> > condition in the section */
> > =C2=A0					if (!(wden & (0x01<<i))) {
> > @@ -1196,10 +1186,9 @@ void rtl8723b_InitBeaconParameters(struct
> > adapter *padapter)
> > =C2=A0{
> > =C2=A0	struct hal_com_data *pHalData =3D GET_HAL_DATA(padapter);
> > =C2=A0	u16 val16;
> > -	u8 val8;
> > +	u8 val8 =3D DIS_TSF_UDT;
> >=20
> >=20
> > -	val8 =3D DIS_TSF_UDT;
> > =C2=A0	val16 =3D val8 | (val8 << 8); /*=C2=A0 port0 and port1 */
> >=20
> > =C2=A0	/*=C2=A0 Enable prot0 beacon function for PSTDMA */
> > @@ -1496,10 +1485,7 @@ s32 rtl8723b_InitLLTTable(struct adapter
> > *padapter)
> > =C2=A0{
> > =C2=A0	unsigned long start, passing_time;
> > =C2=A0	u32 val32;
> > -	s32 ret;
> > -
> > -
> > -	ret =3D _FAIL;
> > +	s32 ret =3D _FAIL;
> >=20
> > =C2=A0	val32 =3D rtw_read32(padapter, REG_AUTO_LLT);
> > =C2=A0	val32 |=3D BIT_AUTO_INIT_LLT;
> > @@ -2273,9 +2259,8 @@ void rtl8723b_fill_fake_txdesc(
> > =C2=A0	/*=C2=A0 Encrypt the data frame if under security mode excepct
> > null data. Suggested by CCW. */
> > =C2=A0	/*=C2=A0 */
> > =C2=A0	if (bDataFrame) {
> > -		u32 EncAlg;
> > +		u32 EncAlg =3D padapter-
> > >securitypriv.dot11PrivacyAlgrthm;
> >=20
> > -		EncAlg =3D padapter-
> > >securitypriv.dot11PrivacyAlgrthm;
> > =C2=A0		switch (EncAlg) {
> > =C2=A0		case _NO_PRIVACY_:
> > =C2=A0			SET_TX_DESC_SEC_TYPE_8723B(pDesc, 0x0);
> > @@ -2378,9 +2363,7 @@ static void hw_var_set_opmode(struct adapter
> > *padapter, u8 variable, u8 *val)
> > =C2=A0static void hw_var_set_macaddr(struct adapter *padapter, u8
> > variable, u8 *val)
> > =C2=A0{
> > =C2=A0	u8 idx =3D 0;
> > -	u32 reg_macid;
> > -
> > -	reg_macid =3D REG_MACID;
> > +	u32 reg_macid =3D REG_MACID;
> >=20
> > =C2=A0	for (idx =3D 0 ; idx < 6; idx++)
> > =C2=A0		rtw_write8(GET_PRIMARY_ADAPTER(padapter),
> > (reg_macid+idx), val[idx]);
> > @@ -2389,9 +2372,7 @@ static void hw_var_set_macaddr(struct adapter
> > *padapter, u8 variable, u8 *val)
> > =C2=A0static void hw_var_set_bssid(struct adapter *padapter, u8
> > variable, u8 *val)
> > =C2=A0{
> > =C2=A0	u8 idx =3D 0;
> > -	u32 reg_bssid;
> > -
> > -	reg_bssid =3D REG_BSSID;
> > +	u32 reg_bssid =3D REG_BSSID;
> >=20
> > =C2=A0	for (idx =3D 0 ; idx < 6; idx++)
> > =C2=A0		rtw_write8(padapter, (reg_bssid+idx), val[idx]);
> > @@ -2399,9 +2380,7 @@ static void hw_var_set_bssid(struct adapter
> > *padapter, u8 variable, u8 *val)
> >=20
> > =C2=A0static void hw_var_set_bcn_func(struct adapter *padapter, u8
> > variable, u8 *val)
> > =C2=A0{
> > -	u32 bcn_ctrl_reg;
> > -
> > -	bcn_ctrl_reg =3D REG_BCN_CTRL;
> > +	u32 bcn_ctrl_reg =3D REG_BCN_CTRL;
> >=20
> > =C2=A0	if (*(u8 *)val)
> > =C2=A0		rtw_write8(padapter, bcn_ctrl_reg,
> > (EN_BCN_FUNCTION | EN_TXBCN_RPT));
> > @@ -2422,12 +2401,8 @@ static void hw_var_set_correct_tsf(struct
> > adapter *padapter, u8 variable, u8 *va
> > =C2=A0{
> > =C2=A0	u8 val8;
> > =C2=A0	u64 tsf;
> > -	struct mlme_ext_priv *pmlmeext;
> > -	struct mlme_ext_info *pmlmeinfo;
> > -
> > -
> > -	pmlmeext =3D &padapter->mlmeextpriv;
> > -	pmlmeinfo =3D &pmlmeext->mlmext_info;
> > +	struct mlme_ext_priv *pmlmeext =3D &padapter->mlmeextpriv;
> > +	struct mlme_ext_info *pmlmeinfo =3D &pmlmeext->mlmext_info;
> >=20
> > =C2=A0	tsf =3D pmlmeext->TSFValue-do_div(pmlmeext->TSFValue,
> > (pmlmeinfo->bcn_interval*1024))-1024; /* us */
> >=20
> > @@ -2543,15 +2518,12 @@ static void hw_var_set_mlme_join(struct
> > adapter *padapter, u8 variable, u8 *val)
> > =C2=A0	u8 val8;
> > =C2=A0	u16 val16;
> > =C2=A0	u32 val32;
> > -	u8 RetryLimit;
> > -	u8 type;
> > -	struct mlme_priv *pmlmepriv;
> > +	u8 RetryLimit =3D 0x30;
> > +	u8 type =3D *(u8 *)val;
> > +	struct mlme_priv *pmlmepriv =3D &padapter->mlmepriv;
> > =C2=A0	struct eeprom_priv *pEEPROM;
> >=20
> >=20
> > -	RetryLimit =3D 0x30;
> > -	type =3D *(u8 *)val;
> > -	pmlmepriv =3D &padapter->mlmepriv;
> > =C2=A0	pEEPROM =3D GET_EEPROM_EFUSE_PRIV(padapter);
> >=20
> > =C2=A0	if (type =3D=3D 0) { /*=C2=A0 prepare to join */
> > @@ -2850,12 +2822,11 @@ void SetHwReg8723B(struct adapter
> > *padapter, u8 variable, u8 *val)
> >=20
> > =C2=A0	case HW_VAR_ACK_PREAMBLE:
> > =C2=A0		{
> > -			u8 regTmp;
> > +			u8 regTmp =3D 0;
> > =C2=A0			u8 bShortPreamble =3D *val;
> >=20
> > =C2=A0			/*=C2=A0 Joseph marked out for Netgear 3500
> > TKIP channel 7 issue.(Temporarily) */
> > =C2=A0			/* regTmp =3D (pHalData-
> > >nCur40MhzPrimeSC)<<5; */
> > -			regTmp =3D 0;
> > =C2=A0			if (bShortPreamble)
> > =C2=A0				regTmp |=3D 0x80;
> > =C2=A0			rtw_write8(padapter, REG_RRSR+2, regTmp);
> > @@ -3226,9 +3197,7 @@ void GetHwReg8723B(struct adapter *padapter,
> > u8 variable, u8 *val)
> > =C2=A0 */
> > =C2=A0u8 SetHalDefVar8723B(struct adapter *padapter, enum
> > hal_def_variable variable, void *pval)
> > =C2=A0{
> > -	u8 bResult;
> > -
> > -	bResult =3D _SUCCESS;
> > +	u8 bResult =3D _SUCCESS;
> >=20
> > =C2=A0	switch (variable) {
> > =C2=A0	default:
> > @@ -3244,9 +3213,7 @@ u8 SetHalDefVar8723B(struct adapter
> > *padapter, enum hal_def_variable variable, v
> > =C2=A0 */
> > =C2=A0u8 GetHalDefVar8723B(struct adapter *padapter, enum
> > hal_def_variable variable, void *pval)
> > =C2=A0{
> > -	u8 bResult;
> > -
> > -	bResult =3D _SUCCESS;
> > +	u8 bResult =3D _SUCCESS;
> >=20
> > =C2=A0	switch (variable) {
> > =C2=A0	case HAL_DEF_MAX_RECVBUF_SZ:
> > @@ -3281,9 +3248,8 @@ u8 GetHalDefVar8723B(struct adapter
> > *padapter, enum hal_def_variable variable, v
> > =C2=A0	case HW_DEF_RA_INFO_DUMP:
> > =C2=A0		{
> > =C2=A0			u8 mac_id =3D *(u8 *)pval;
> > -			u32 cmd;
> > +			u32 cmd =3D 0x40000100 | mac_id;
> >=20
> > -			cmd =3D 0x40000100 | mac_id;
> > =C2=A0			rtw_write32(padapter,
> > REG_HMEBOX_DBG_2_8723B, cmd);
> > =C2=A0			msleep(10);
> > =C2=A0			rtw_read32(padapter, 0x2F0);	// info 1
> > --
> > 2.43.0
> >=20
> >=20
> >=20