[PATCH v2 0/2] Improve code readability in rtl8723bs module

[PATCH v2 0/2] Improve code readability in rtl8723bs module

[Erick Karanja]

The patchset aims at improving code readability by initializing
variables at declaration.Key consideration is made to ensure that the
code is clean, maintainable and easy to debug.

changes since v1:
Avoid mixing pre-initialized variables with not-initialized variables
and handle an edge case where initialization is made to function call.

Erick Karanja (2):
  staging: rtl8723bs: Improve code readability
  staging: rtl8723bs: Initializing variables at declaration

 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 84 ++++++-------------
 .../staging/rtl8723bs/hal/rtl8723bs_xmit.c    | 39 +++------
 2 files changed, 36 insertions(+), 87 deletions(-)

-- 
2.43.0

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

[Erick Karanja]

Make the code more readable by moving trivial
initializations up with the declarations instead
of wasting a line on that.

Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
 .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 84 ++++++-------------
 1 file changed, 25 insertions(+), 59 deletions(-)

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(
 	switch (type) {
 	case TYPE_EFUSE_MAX_SECTION:
 		{
-			u8 *pMax_section;
-			pMax_section = pOut;
+			u8 *pMax_section = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pMax_section = EFUSE_MAX_SECTION_8723B;
@@ -513,8 +512,7 @@ void Hal_GetEfuseDefinition(
 
 	case TYPE_EFUSE_REAL_CONTENT_LEN:
 		{
-			u16 *pu2Tmp;
-			pu2Tmp = pOut;
+			u16 *pu2Tmp = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pu2Tmp = EFUSE_REAL_CONTENT_LEN_8723B;
@@ -525,8 +523,7 @@ void Hal_GetEfuseDefinition(
 
 	case TYPE_AVAILABLE_EFUSE_BYTES_BANK:
 		{
-			u16 *pu2Tmp;
-			pu2Tmp = pOut;
+			u16 *pu2Tmp = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pu2Tmp = (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
@@ -537,8 +534,7 @@ void Hal_GetEfuseDefinition(
 
 	case TYPE_AVAILABLE_EFUSE_BYTES_TOTAL:
 		{
-			u16 *pu2Tmp;
-			pu2Tmp = pOut;
+			u16 *pu2Tmp = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pu2Tmp = (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
@@ -549,8 +545,7 @@ void Hal_GetEfuseDefinition(
 
 	case TYPE_EFUSE_MAP_LEN:
 		{
-			u16 *pu2Tmp;
-			pu2Tmp = pOut;
+			u16 *pu2Tmp = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pu2Tmp = EFUSE_MAX_MAP_LEN;
@@ -561,8 +556,7 @@ void Hal_GetEfuseDefinition(
 
 	case TYPE_EFUSE_PROTECT_BYTES_BANK:
 		{
-			u8 *pu1Tmp;
-			pu1Tmp = pOut;
+			u8 *pu1Tmp = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pu1Tmp = EFUSE_OOB_PROTECT_BYTES;
@@ -573,8 +567,7 @@ void Hal_GetEfuseDefinition(
 
 	case TYPE_EFUSE_CONTENT_LEN_BANK:
 		{
-			u16 *pu2Tmp;
-			pu2Tmp = pOut;
+			u16 *pu2Tmp = pOut;
 
 			if (efuseType == EFUSE_WIFI)
 				*pu2Tmp = EFUSE_REAL_CONTENT_LEN_8723B;
@@ -585,8 +578,7 @@ void Hal_GetEfuseDefinition(
 
 	default:
 		{
-			u8 *pu1Tmp;
-			pu1Tmp = pOut;
+			u8 *pu1Tmp = pOut;
 			*pu1Tmp = 0;
 		}
 		break;
@@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
 		}
 
 		if (offset < EFUSE_MAX_SECTION_8723B) {
-			u16 addr;
+			u16 addr = offset * PGPKT_DATA_SIZE;
 			/*  Get word enable value from PG header */
 
-			addr = offset * PGPKT_DATA_SIZE;
 			for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
 				/*  Check word enable condition in the section */
 				if (!(wden & (0x01<<i))) {
@@ -835,9 +826,8 @@ static void hal_ReadEFuse_BT(
 			}
 
 			if (offset < EFUSE_BT_MAX_SECTION) {
-				u16 addr;
+				u16 addr = offset * PGPKT_DATA_SIZE;
 
-				addr = offset * PGPKT_DATA_SIZE;
 				for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
 					/*  Check word enable condition in the section */
 					if (!(wden & (0x01<<i))) {
@@ -1196,10 +1186,9 @@ void rtl8723b_InitBeaconParameters(struct adapter *padapter)
 {
 	struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
 	u16 val16;
-	u8 val8;
+	u8 val8 = DIS_TSF_UDT;
 
 
-	val8 = DIS_TSF_UDT;
 	val16 = val8 | (val8 << 8); /*  port0 and port1 */
 
 	/*  Enable prot0 beacon function for PSTDMA */
@@ -1496,10 +1485,7 @@ s32 rtl8723b_InitLLTTable(struct adapter *padapter)
 {
 	unsigned long start, passing_time;
 	u32 val32;
-	s32 ret;
-
-
-	ret = _FAIL;
+	s32 ret = _FAIL;
 
 	val32 = rtw_read32(padapter, REG_AUTO_LLT);
 	val32 |= BIT_AUTO_INIT_LLT;
@@ -2273,9 +2259,8 @@ void rtl8723b_fill_fake_txdesc(
 	/*  Encrypt the data frame if under security mode excepct null data. Suggested by CCW. */
 	/*  */
 	if (bDataFrame) {
-		u32 EncAlg;
+		u32 EncAlg = padapter->securitypriv.dot11PrivacyAlgrthm;
 
-		EncAlg = padapter->securitypriv.dot11PrivacyAlgrthm;
 		switch (EncAlg) {
 		case _NO_PRIVACY_:
 			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)
 static void hw_var_set_macaddr(struct adapter *padapter, u8 variable, u8 *val)
 {
 	u8 idx = 0;
-	u32 reg_macid;
-
-	reg_macid = REG_MACID;
+	u32 reg_macid = REG_MACID;
 
 	for (idx = 0 ; idx < 6; idx++)
 		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)
 static void hw_var_set_bssid(struct adapter *padapter, u8 variable, u8 *val)
 {
 	u8 idx = 0;
-	u32 reg_bssid;
-
-	reg_bssid = REG_BSSID;
+	u32 reg_bssid = REG_BSSID;
 
 	for (idx = 0 ; idx < 6; idx++)
 		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)
 
 static void hw_var_set_bcn_func(struct adapter *padapter, u8 variable, u8 *val)
 {
-	u32 bcn_ctrl_reg;
-
-	bcn_ctrl_reg = REG_BCN_CTRL;
+	u32 bcn_ctrl_reg = REG_BCN_CTRL;
 
 	if (*(u8 *)val)
 		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
 {
 	u8 val8;
 	u64 tsf;
-	struct mlme_ext_priv *pmlmeext;
-	struct mlme_ext_info *pmlmeinfo;
-
-
-	pmlmeext = &padapter->mlmeextpriv;
-	pmlmeinfo = &pmlmeext->mlmext_info;
+	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
+	struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
 
 	tsf = pmlmeext->TSFValue-do_div(pmlmeext->TSFValue, (pmlmeinfo->bcn_interval*1024))-1024; /* us */
 
@@ -2543,15 +2518,12 @@ static void hw_var_set_mlme_join(struct adapter *padapter, u8 variable, u8 *val)
 	u8 val8;
 	u16 val16;
 	u32 val32;
-	u8 RetryLimit;
-	u8 type;
-	struct mlme_priv *pmlmepriv;
+	u8 RetryLimit = 0x30;
+	u8 type = *(u8 *)val;
+	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct eeprom_priv *pEEPROM;
 
 
-	RetryLimit = 0x30;
-	type = *(u8 *)val;
-	pmlmepriv = &padapter->mlmepriv;
 	pEEPROM = GET_EEPROM_EFUSE_PRIV(padapter);
 
 	if (type == 0) { /*  prepare to join */
@@ -2850,12 +2822,11 @@ void SetHwReg8723B(struct adapter *padapter, u8 variable, u8 *val)
 
 	case HW_VAR_ACK_PREAMBLE:
 		{
-			u8 regTmp;
+			u8 regTmp = 0;
 			u8 bShortPreamble = *val;
 
 			/*  Joseph marked out for Netgear 3500 TKIP channel 7 issue.(Temporarily) */
 			/* regTmp = (pHalData->nCur40MhzPrimeSC)<<5; */
-			regTmp = 0;
 			if (bShortPreamble)
 				regTmp |= 0x80;
 			rtw_write8(padapter, REG_RRSR+2, regTmp);
@@ -3226,9 +3197,7 @@ void GetHwReg8723B(struct adapter *padapter, u8 variable, u8 *val)
  */
 u8 SetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, void *pval)
 {
-	u8 bResult;
-
-	bResult = _SUCCESS;
+	u8 bResult = _SUCCESS;
 
 	switch (variable) {
 	default:
@@ -3244,9 +3213,7 @@ u8 SetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, v
  */
 u8 GetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, void *pval)
 {
-	u8 bResult;
-
-	bResult = _SUCCESS;
+	u8 bResult = _SUCCESS;
 
 	switch (variable) {
 	case HAL_DEF_MAX_RECVBUF_SZ:
@@ -3281,9 +3248,8 @@ u8 GetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, v
 	case HW_DEF_RA_INFO_DUMP:
 		{
 			u8 mac_id = *(u8 *)pval;
-			u32 cmd;
+			u32 cmd = 0x40000100 | mac_id;
 
-			cmd = 0x40000100 | mac_id;
 			rtw_write32(padapter, REG_HMEBOX_DBG_2_8723B, cmd);
 			msleep(10);
 			rtw_read32(padapter, 0x2F0);	// info 1
-- 
2.43.0

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

[Julia Lawall]

On Wed, 9 Apr 2025, Erick Karanja wrote:

> Make the code more readable by moving trivial
> initializations up with the declarations instead
> of wasting a line on that.
>
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
>  .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 84 ++++++-------------
>  1 file changed, 25 insertions(+), 59 deletions(-)
>
> 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(
>  	switch (type) {
>  	case TYPE_EFUSE_MAX_SECTION:
>  		{
> -			u8 *pMax_section;
> -			pMax_section = pOut;
> +			u8 *pMax_section = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pMax_section = EFUSE_MAX_SECTION_8723B;
> @@ -513,8 +512,7 @@ void Hal_GetEfuseDefinition(
>
>  	case TYPE_EFUSE_REAL_CONTENT_LEN:
>  		{
> -			u16 *pu2Tmp;
> -			pu2Tmp = pOut;
> +			u16 *pu2Tmp = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pu2Tmp = EFUSE_REAL_CONTENT_LEN_8723B;
> @@ -525,8 +523,7 @@ void Hal_GetEfuseDefinition(
>
>  	case TYPE_AVAILABLE_EFUSE_BYTES_BANK:
>  		{
> -			u16 *pu2Tmp;
> -			pu2Tmp = pOut;
> +			u16 *pu2Tmp = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pu2Tmp = (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> @@ -537,8 +534,7 @@ void Hal_GetEfuseDefinition(
>
>  	case TYPE_AVAILABLE_EFUSE_BYTES_TOTAL:
>  		{
> -			u16 *pu2Tmp;
> -			pu2Tmp = pOut;
> +			u16 *pu2Tmp = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pu2Tmp = (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> @@ -549,8 +545,7 @@ void Hal_GetEfuseDefinition(
>
>  	case TYPE_EFUSE_MAP_LEN:
>  		{
> -			u16 *pu2Tmp;
> -			pu2Tmp = pOut;
> +			u16 *pu2Tmp = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pu2Tmp = EFUSE_MAX_MAP_LEN;
> @@ -561,8 +556,7 @@ void Hal_GetEfuseDefinition(
>
>  	case TYPE_EFUSE_PROTECT_BYTES_BANK:
>  		{
> -			u8 *pu1Tmp;
> -			pu1Tmp = pOut;
> +			u8 *pu1Tmp = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pu1Tmp = EFUSE_OOB_PROTECT_BYTES;
> @@ -573,8 +567,7 @@ void Hal_GetEfuseDefinition(
>
>  	case TYPE_EFUSE_CONTENT_LEN_BANK:
>  		{
> -			u16 *pu2Tmp;
> -			pu2Tmp = pOut;
> +			u16 *pu2Tmp = pOut;
>
>  			if (efuseType == EFUSE_WIFI)
>  				*pu2Tmp = EFUSE_REAL_CONTENT_LEN_8723B;
> @@ -585,8 +578,7 @@ void Hal_GetEfuseDefinition(
>
>  	default:
>  		{
> -			u8 *pu1Tmp;
> -			pu1Tmp = pOut;
> +			u8 *pu1Tmp = pOut;
>  			*pu1Tmp = 0;
>  		}
>  		break;
> @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
>  		}
>
>  		if (offset < EFUSE_MAX_SECTION_8723B) {
> -			u16 addr;
> +			u16 addr = offset * PGPKT_DATA_SIZE;
>  			/*  Get word enable value from PG header */
>
> -			addr = offset * PGPKT_DATA_SIZE;

I'm not sure about this one, due to the comment.

julia

>  			for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
>  				/*  Check word enable condition in the section */
>  				if (!(wden & (0x01<<i))) {
> @@ -835,9 +826,8 @@ static void hal_ReadEFuse_BT(
>  			}
>
>  			if (offset < EFUSE_BT_MAX_SECTION) {
> -				u16 addr;
> +				u16 addr = offset * PGPKT_DATA_SIZE;
>
> -				addr = offset * PGPKT_DATA_SIZE;
>  				for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
>  					/*  Check word enable condition in the section */
>  					if (!(wden & (0x01<<i))) {
> @@ -1196,10 +1186,9 @@ void rtl8723b_InitBeaconParameters(struct adapter *padapter)
>  {
>  	struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
>  	u16 val16;
> -	u8 val8;
> +	u8 val8 = DIS_TSF_UDT;
>
>
> -	val8 = DIS_TSF_UDT;
>  	val16 = val8 | (val8 << 8); /*  port0 and port1 */
>
>  	/*  Enable prot0 beacon function for PSTDMA */
> @@ -1496,10 +1485,7 @@ s32 rtl8723b_InitLLTTable(struct adapter *padapter)
>  {
>  	unsigned long start, passing_time;
>  	u32 val32;
> -	s32 ret;
> -
> -
> -	ret = _FAIL;
> +	s32 ret = _FAIL;
>
>  	val32 = rtw_read32(padapter, REG_AUTO_LLT);
>  	val32 |= BIT_AUTO_INIT_LLT;
> @@ -2273,9 +2259,8 @@ void rtl8723b_fill_fake_txdesc(
>  	/*  Encrypt the data frame if under security mode excepct null data. Suggested by CCW. */
>  	/*  */
>  	if (bDataFrame) {
> -		u32 EncAlg;
> +		u32 EncAlg = padapter->securitypriv.dot11PrivacyAlgrthm;
>
> -		EncAlg = padapter->securitypriv.dot11PrivacyAlgrthm;
>  		switch (EncAlg) {
>  		case _NO_PRIVACY_:
>  			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)
>  static void hw_var_set_macaddr(struct adapter *padapter, u8 variable, u8 *val)
>  {
>  	u8 idx = 0;
> -	u32 reg_macid;
> -
> -	reg_macid = REG_MACID;
> +	u32 reg_macid = REG_MACID;
>
>  	for (idx = 0 ; idx < 6; idx++)
>  		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)
>  static void hw_var_set_bssid(struct adapter *padapter, u8 variable, u8 *val)
>  {
>  	u8 idx = 0;
> -	u32 reg_bssid;
> -
> -	reg_bssid = REG_BSSID;
> +	u32 reg_bssid = REG_BSSID;
>
>  	for (idx = 0 ; idx < 6; idx++)
>  		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)
>
>  static void hw_var_set_bcn_func(struct adapter *padapter, u8 variable, u8 *val)
>  {
> -	u32 bcn_ctrl_reg;
> -
> -	bcn_ctrl_reg = REG_BCN_CTRL;
> +	u32 bcn_ctrl_reg = REG_BCN_CTRL;
>
>  	if (*(u8 *)val)
>  		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
>  {
>  	u8 val8;
>  	u64 tsf;
> -	struct mlme_ext_priv *pmlmeext;
> -	struct mlme_ext_info *pmlmeinfo;
> -
> -
> -	pmlmeext = &padapter->mlmeextpriv;
> -	pmlmeinfo = &pmlmeext->mlmext_info;
> +	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> +	struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
>
>  	tsf = pmlmeext->TSFValue-do_div(pmlmeext->TSFValue, (pmlmeinfo->bcn_interval*1024))-1024; /* us */
>
> @@ -2543,15 +2518,12 @@ static void hw_var_set_mlme_join(struct adapter *padapter, u8 variable, u8 *val)
>  	u8 val8;
>  	u16 val16;
>  	u32 val32;
> -	u8 RetryLimit;
> -	u8 type;
> -	struct mlme_priv *pmlmepriv;
> +	u8 RetryLimit = 0x30;
> +	u8 type = *(u8 *)val;
> +	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>  	struct eeprom_priv *pEEPROM;
>
>
> -	RetryLimit = 0x30;
> -	type = *(u8 *)val;
> -	pmlmepriv = &padapter->mlmepriv;
>  	pEEPROM = GET_EEPROM_EFUSE_PRIV(padapter);
>
>  	if (type == 0) { /*  prepare to join */
> @@ -2850,12 +2822,11 @@ void SetHwReg8723B(struct adapter *padapter, u8 variable, u8 *val)
>
>  	case HW_VAR_ACK_PREAMBLE:
>  		{
> -			u8 regTmp;
> +			u8 regTmp = 0;
>  			u8 bShortPreamble = *val;
>
>  			/*  Joseph marked out for Netgear 3500 TKIP channel 7 issue.(Temporarily) */
>  			/* regTmp = (pHalData->nCur40MhzPrimeSC)<<5; */
> -			regTmp = 0;
>  			if (bShortPreamble)
>  				regTmp |= 0x80;
>  			rtw_write8(padapter, REG_RRSR+2, regTmp);
> @@ -3226,9 +3197,7 @@ void GetHwReg8723B(struct adapter *padapter, u8 variable, u8 *val)
>   */
>  u8 SetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, void *pval)
>  {
> -	u8 bResult;
> -
> -	bResult = _SUCCESS;
> +	u8 bResult = _SUCCESS;
>
>  	switch (variable) {
>  	default:
> @@ -3244,9 +3213,7 @@ u8 SetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, v
>   */
>  u8 GetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, void *pval)
>  {
> -	u8 bResult;
> -
> -	bResult = _SUCCESS;
> +	u8 bResult = _SUCCESS;
>
>  	switch (variable) {
>  	case HAL_DEF_MAX_RECVBUF_SZ:
> @@ -3281,9 +3248,8 @@ u8 GetHalDefVar8723B(struct adapter *padapter, enum hal_def_variable variable, v
>  	case HW_DEF_RA_INFO_DUMP:
>  		{
>  			u8 mac_id = *(u8 *)pval;
> -			u32 cmd;
> +			u32 cmd = 0x40000100 | mac_id;
>
> -			cmd = 0x40000100 | mac_id;
>  			rtw_write32(padapter, REG_HMEBOX_DBG_2_8723B, cmd);
>  			msleep(10);
>  			rtw_read32(padapter, 0x2F0);	// info 1
> --
> 2.43.0
>
>
>

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

[Dan Carpenter]

On Wed, Apr 09, 2025 at 07:04:18AM -0400, Julia Lawall wrote:
> > @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
> >  		}
> >
> >  		if (offset < EFUSE_MAX_SECTION_8723B) {
> > -			u16 addr;
> > +			u16 addr = offset * PGPKT_DATA_SIZE;
> >  			/*  Get word enable value from PG header */
> >
> > -			addr = offset * PGPKT_DATA_SIZE;
> 
> I'm not sure about this one, due to the comment.
> 
> julia
> 

I feel like it works.  I wish there were a blank line after the
declaration but that's something for a different patch.  Both of
these are okay with me.  Try to be more creative with the subjects
next time.  (Just put the file name in the subject because that's
the only difference).

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter

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

[Erick Karanja]

On Wed, 2025-04-09 at 14:11 +0300, Dan Carpenter wrote:
> On Wed, Apr 09, 2025 at 07:04:18AM -0400, Julia Lawall wrote:
> > > @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
> > >  		}
> > > 
> > >  		if (offset < EFUSE_MAX_SECTION_8723B) {
> > > -			u16 addr;
> > > +			u16 addr = offset * PGPKT_DATA_SIZE;
> > >  			/*  Get word enable value from PG header
> > > */
> > > 
> > > -			addr = offset * PGPKT_DATA_SIZE;
> > 
> > I'm not sure about this one, due to the comment.
> > 
> > julia
> > 
> 
> I feel like it works.  I wish there were a blank line after the
> declaration but that's something for a different patch.  Both of
> these are okay with me.  Try to be more creative with the subjects
> next time.  (Just put the file name in the subject because that's
> the only difference).
Thank you for the review.
Erick
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> regards,
> dan carpenter
> 

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

[Erick Karanja]

On Wed, 2025-04-09 at 07:04 -0400, Julia Lawall wrote:
> 
> 
> On Wed, 9 Apr 2025, Erick Karanja wrote:
> 
> > Make the code more readable by moving trivial
> > initializations up with the declarations instead
> > of wasting a line on that.
> > 
> > Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> > ---
> >  .../staging/rtl8723bs/hal/rtl8723b_hal_init.c | 84 ++++++---------
> > ----
> >  1 file changed, 25 insertions(+), 59 deletions(-)
> > 
> > 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(
> >  	switch (type) {
> >  	case TYPE_EFUSE_MAX_SECTION:
> >  		{
> > -			u8 *pMax_section;
> > -			pMax_section = pOut;
> > +			u8 *pMax_section = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pMax_section =
> > EFUSE_MAX_SECTION_8723B;
> > @@ -513,8 +512,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	case TYPE_EFUSE_REAL_CONTENT_LEN:
> >  		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp = pOut;
> > +			u16 *pu2Tmp = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pu2Tmp =
> > EFUSE_REAL_CONTENT_LEN_8723B;
> > @@ -525,8 +523,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	case TYPE_AVAILABLE_EFUSE_BYTES_BANK:
> >  		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp = pOut;
> > +			u16 *pu2Tmp = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pu2Tmp =
> > (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> > @@ -537,8 +534,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	case TYPE_AVAILABLE_EFUSE_BYTES_TOTAL:
> >  		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp = pOut;
> > +			u16 *pu2Tmp = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pu2Tmp =
> > (EFUSE_REAL_CONTENT_LEN_8723B-EFUSE_OOB_PROTECT_BYTES);
> > @@ -549,8 +545,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	case TYPE_EFUSE_MAP_LEN:
> >  		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp = pOut;
> > +			u16 *pu2Tmp = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pu2Tmp = EFUSE_MAX_MAP_LEN;
> > @@ -561,8 +556,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	case TYPE_EFUSE_PROTECT_BYTES_BANK:
> >  		{
> > -			u8 *pu1Tmp;
> > -			pu1Tmp = pOut;
> > +			u8 *pu1Tmp = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pu1Tmp = EFUSE_OOB_PROTECT_BYTES;
> > @@ -573,8 +567,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	case TYPE_EFUSE_CONTENT_LEN_BANK:
> >  		{
> > -			u16 *pu2Tmp;
> > -			pu2Tmp = pOut;
> > +			u16 *pu2Tmp = pOut;
> > 
> >  			if (efuseType == EFUSE_WIFI)
> >  				*pu2Tmp =
> > EFUSE_REAL_CONTENT_LEN_8723B;
> > @@ -585,8 +578,7 @@ void Hal_GetEfuseDefinition(
> > 
> >  	default:
> >  		{
> > -			u8 *pu1Tmp;
> > -			pu1Tmp = pOut;
> > +			u8 *pu1Tmp = pOut;
> >  			*pu1Tmp = 0;
> >  		}
> >  		break;
> > @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
> >  		}
> > 
> >  		if (offset < EFUSE_MAX_SECTION_8723B) {
> > -			u16 addr;
> > +			u16 addr = offset * PGPKT_DATA_SIZE;
> >  			/*  Get word enable value from PG header
> > */
> > 
> > -			addr = offset * PGPKT_DATA_SIZE;
> 
> 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. 
> 
> julia
> 
> >  			for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++)
> > {
> >  				/*  Check word enable condition in
> > the section */
> >  				if (!(wden & (0x01<<i))) {
> > @@ -835,9 +826,8 @@ static void hal_ReadEFuse_BT(
> >  			}
> > 
> >  			if (offset < EFUSE_BT_MAX_SECTION) {
> > -				u16 addr;
> > +				u16 addr = offset *
> > PGPKT_DATA_SIZE;
> > 
> > -				addr = offset * PGPKT_DATA_SIZE;
> >  				for (i = 0; i <
> > EFUSE_MAX_WORD_UNIT; i++) {
> >  					/*  Check word enable
> > condition in the section */
> >  					if (!(wden & (0x01<<i))) {
> > @@ -1196,10 +1186,9 @@ void rtl8723b_InitBeaconParameters(struct
> > adapter *padapter)
> >  {
> >  	struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
> >  	u16 val16;
> > -	u8 val8;
> > +	u8 val8 = DIS_TSF_UDT;
> > 
> > 
> > -	val8 = DIS_TSF_UDT;
> >  	val16 = val8 | (val8 << 8); /*  port0 and port1 */
> > 
> >  	/*  Enable prot0 beacon function for PSTDMA */
> > @@ -1496,10 +1485,7 @@ s32 rtl8723b_InitLLTTable(struct adapter
> > *padapter)
> >  {
> >  	unsigned long start, passing_time;
> >  	u32 val32;
> > -	s32 ret;
> > -
> > -
> > -	ret = _FAIL;
> > +	s32 ret = _FAIL;
> > 
> >  	val32 = rtw_read32(padapter, REG_AUTO_LLT);
> >  	val32 |= BIT_AUTO_INIT_LLT;
> > @@ -2273,9 +2259,8 @@ void rtl8723b_fill_fake_txdesc(
> >  	/*  Encrypt the data frame if under security mode excepct
> > null data. Suggested by CCW. */
> >  	/*  */
> >  	if (bDataFrame) {
> > -		u32 EncAlg;
> > +		u32 EncAlg = padapter-
> > >securitypriv.dot11PrivacyAlgrthm;
> > 
> > -		EncAlg = padapter-
> > >securitypriv.dot11PrivacyAlgrthm;
> >  		switch (EncAlg) {
> >  		case _NO_PRIVACY_:
> >  			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)
> >  static void hw_var_set_macaddr(struct adapter *padapter, u8
> > variable, u8 *val)
> >  {
> >  	u8 idx = 0;
> > -	u32 reg_macid;
> > -
> > -	reg_macid = REG_MACID;
> > +	u32 reg_macid = REG_MACID;
> > 
> >  	for (idx = 0 ; idx < 6; idx++)
> >  		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)
> >  static void hw_var_set_bssid(struct adapter *padapter, u8
> > variable, u8 *val)
> >  {
> >  	u8 idx = 0;
> > -	u32 reg_bssid;
> > -
> > -	reg_bssid = REG_BSSID;
> > +	u32 reg_bssid = REG_BSSID;
> > 
> >  	for (idx = 0 ; idx < 6; idx++)
> >  		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)
> > 
> >  static void hw_var_set_bcn_func(struct adapter *padapter, u8
> > variable, u8 *val)
> >  {
> > -	u32 bcn_ctrl_reg;
> > -
> > -	bcn_ctrl_reg = REG_BCN_CTRL;
> > +	u32 bcn_ctrl_reg = REG_BCN_CTRL;
> > 
> >  	if (*(u8 *)val)
> >  		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
> >  {
> >  	u8 val8;
> >  	u64 tsf;
> > -	struct mlme_ext_priv *pmlmeext;
> > -	struct mlme_ext_info *pmlmeinfo;
> > -
> > -
> > -	pmlmeext = &padapter->mlmeextpriv;
> > -	pmlmeinfo = &pmlmeext->mlmext_info;
> > +	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> > +	struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
> > 
> >  	tsf = pmlmeext->TSFValue-do_div(pmlmeext->TSFValue,
> > (pmlmeinfo->bcn_interval*1024))-1024; /* us */
> > 
> > @@ -2543,15 +2518,12 @@ static void hw_var_set_mlme_join(struct
> > adapter *padapter, u8 variable, u8 *val)
> >  	u8 val8;
> >  	u16 val16;
> >  	u32 val32;
> > -	u8 RetryLimit;
> > -	u8 type;
> > -	struct mlme_priv *pmlmepriv;
> > +	u8 RetryLimit = 0x30;
> > +	u8 type = *(u8 *)val;
> > +	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> >  	struct eeprom_priv *pEEPROM;
> > 
> > 
> > -	RetryLimit = 0x30;
> > -	type = *(u8 *)val;
> > -	pmlmepriv = &padapter->mlmepriv;
> >  	pEEPROM = GET_EEPROM_EFUSE_PRIV(padapter);
> > 
> >  	if (type == 0) { /*  prepare to join */
> > @@ -2850,12 +2822,11 @@ void SetHwReg8723B(struct adapter
> > *padapter, u8 variable, u8 *val)
> > 
> >  	case HW_VAR_ACK_PREAMBLE:
> >  		{
> > -			u8 regTmp;
> > +			u8 regTmp = 0;
> >  			u8 bShortPreamble = *val;
> > 
> >  			/*  Joseph marked out for Netgear 3500
> > TKIP channel 7 issue.(Temporarily) */
> >  			/* regTmp = (pHalData-
> > >nCur40MhzPrimeSC)<<5; */
> > -			regTmp = 0;
> >  			if (bShortPreamble)
> >  				regTmp |= 0x80;
> >  			rtw_write8(padapter, REG_RRSR+2, regTmp);
> > @@ -3226,9 +3197,7 @@ void GetHwReg8723B(struct adapter *padapter,
> > u8 variable, u8 *val)
> >   */
> >  u8 SetHalDefVar8723B(struct adapter *padapter, enum
> > hal_def_variable variable, void *pval)
> >  {
> > -	u8 bResult;
> > -
> > -	bResult = _SUCCESS;
> > +	u8 bResult = _SUCCESS;
> > 
> >  	switch (variable) {
> >  	default:
> > @@ -3244,9 +3213,7 @@ u8 SetHalDefVar8723B(struct adapter
> > *padapter, enum hal_def_variable variable, v
> >   */
> >  u8 GetHalDefVar8723B(struct adapter *padapter, enum
> > hal_def_variable variable, void *pval)
> >  {
> > -	u8 bResult;
> > -
> > -	bResult = _SUCCESS;
> > +	u8 bResult = _SUCCESS;
> > 
> >  	switch (variable) {
> >  	case HAL_DEF_MAX_RECVBUF_SZ:
> > @@ -3281,9 +3248,8 @@ u8 GetHalDefVar8723B(struct adapter
> > *padapter, enum hal_def_variable variable, v
> >  	case HW_DEF_RA_INFO_DUMP:
> >  		{
> >  			u8 mac_id = *(u8 *)pval;
> > -			u32 cmd;
> > +			u32 cmd = 0x40000100 | mac_id;
> > 
> > -			cmd = 0x40000100 | mac_id;
> >  			rtw_write32(padapter,
> > REG_HMEBOX_DBG_2_8723B, cmd);
> >  			msleep(10);
> >  			rtw_read32(padapter, 0x2F0);	// info 1
> > --
> > 2.43.0
> > 
> > 
> > 

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

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

> > > @@ -729,10 +721,9 @@ static void hal_ReadEFuse_WiFi(
> > >  		}
> > >
> > >  		if (offset < EFUSE_MAX_SECTION_8723B) {
> > > -			u16 addr;
> > > +			u16 addr = offset * PGPKT_DATA_SIZE;
> > >  			/*  Get word enable value from PG header
> > > */
> > >
> > > -			addr = offset * PGPKT_DATA_SIZE;
> >
> > 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.

Making the right choice depends on the semantics of the comment, which is
out of the scope of Coccinelle.  Generally, if a complete block of code is
dropped, any comments within it are dropped as well.  Here the comment is
kept, which seems reasonable.  The issue is that you may have moved code
that is relevant to the comment up above the comment.  Ultimately, you have
to decide what to do and what not to do.  If you let Coccinelle make a
patch, and then apply the patch, in the meantime you can just remove the
parts of the patch you don't like.  If you make the changes in place, you
can use git add -p to add the changes you like, and not add the others, and
then use git checkout to remove the changes you don't want to do.  I would
take the former approach, but some people may take the latter.

julia

[PATCH v2 2/2] staging: rtl8723bs: Initializing variables at declaration

[Erick Karanja]

Make the code more readable by moving trivial
initializations up with the declarations instead
of wasting a line on that.

Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
 .../staging/rtl8723bs/hal/rtl8723bs_xmit.c    | 39 ++++++-------------
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
index 5dc1c12fe03e..d134d185bfae 100644
--- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
+++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
@@ -120,13 +120,10 @@ static s32 rtl8723_dequeue_writeport(struct adapter *padapter)
  */
 s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
 {
-	struct xmit_priv *pxmitpriv;
+	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 	u8 queue_empty, queue_pending;
 	s32 ret;
 
-
-	pxmitpriv = &padapter->xmitpriv;
-
 	if (wait_for_completion_interruptible(&pxmitpriv->xmit_comp)) {
 		netdev_emerg(padapter->pnetdev,
 			     "%s: down SdioXmitBufSema fail!\n", __func__);
@@ -242,8 +239,7 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
 						/* pxmitbuf->priv_data will be NULL, and will crash here */
 						if (pxmitbuf->len > 0 &&
 						    pxmitbuf->priv_data) {
-							struct xmit_frame *pframe;
-							pframe = (struct xmit_frame *)pxmitbuf->priv_data;
+							struct xmit_frame *pframe = (struct xmit_frame *)pxmitbuf->priv_data;
 							pframe->agg_num = k;
 							pxmitbuf->agg_num = k;
 							rtl8723b_update_txdesc(pframe, pframe->buf_addr);
@@ -326,8 +322,7 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
 		/*  dump xmit_buf to hw tx fifo */
 		if (pxmitbuf) {
 			if (pxmitbuf->len > 0) {
-				struct xmit_frame *pframe;
-				pframe = (struct xmit_frame *)pxmitbuf->priv_data;
+				struct xmit_frame *pframe = (struct xmit_frame *)pxmitbuf->priv_data;
 				pframe->agg_num = k;
 				pxmitbuf->agg_num = k;
 				rtl8723b_update_txdesc(pframe, pframe->buf_addr);
@@ -357,12 +352,9 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
  */
 static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
 {
-	struct xmit_priv *pxmitpriv;
+	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 	s32 ret;
 
-
-	pxmitpriv = &padapter->xmitpriv;
-
 	if (wait_for_completion_interruptible(&pxmitpriv->SdioXmitStart)) {
 		netdev_emerg(padapter->pnetdev, "%s: SdioXmitStart fail!\n",
 			     __func__);
@@ -408,13 +400,9 @@ static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
 
 int rtl8723bs_xmit_thread(void *context)
 {
-	s32 ret;
-	struct adapter *padapter;
-	struct xmit_priv *pxmitpriv;
-
-	ret = _SUCCESS;
-	padapter = context;
-	pxmitpriv = &padapter->xmitpriv;
+	s32 ret = _SUCCESS;
+	struct adapter *padapter = context;
+	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 
 	allow_signal(SIGTERM);
 
@@ -435,16 +423,13 @@ s32 rtl8723bs_mgnt_xmit(
 )
 {
 	s32 ret = _SUCCESS;
-	struct pkt_attrib *pattrib;
-	struct xmit_buf *pxmitbuf;
+	struct pkt_attrib *pattrib = &pmgntframe->attrib;
+	struct xmit_buf *pxmitbuf = pmgntframe->pxmitbuf;
 	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
 	u8 *pframe = (u8 *)(pmgntframe->buf_addr) + TXDESC_OFFSET;
 	u8 txdesc_size = TXDESC_SIZE;
 
-	pattrib = &pmgntframe->attrib;
-	pxmitbuf = pmgntframe->pxmitbuf;
-
 	rtl8723b_update_txdesc(pmgntframe, pmgntframe->buf_addr);
 
 	pxmitbuf->len = txdesc_size + pattrib->last_txcmdsz;
@@ -557,15 +542,13 @@ s32 rtl8723bs_init_xmit_priv(struct adapter *padapter)
 
 void rtl8723bs_free_xmit_priv(struct adapter *padapter)
 {
-	struct xmit_priv *pxmitpriv;
+	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
 	struct xmit_buf *pxmitbuf;
-	struct __queue *pqueue;
+	struct __queue *pqueue = &pxmitpriv->pending_xmitbuf_queue;
 	struct list_head *plist, *phead;
 	struct list_head tmplist;
 
 
-	pxmitpriv = &padapter->xmitpriv;
-	pqueue = &pxmitpriv->pending_xmitbuf_queue;
 	phead = get_list_head(pqueue);
 	INIT_LIST_HEAD(&tmplist);
 
-- 
2.43.0

Re: [PATCH v2 2/2] staging: rtl8723bs: Initializing variables at declaration

[Julia Lawall]

On Wed, 9 Apr 2025, Erick Karanja wrote:

> Make the code more readable by moving trivial
> initializations up with the declarations instead
> of wasting a line on that.
>
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
>  .../staging/rtl8723bs/hal/rtl8723bs_xmit.c    | 39 ++++++-------------
>  1 file changed, 11 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> index 5dc1c12fe03e..d134d185bfae 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> @@ -120,13 +120,10 @@ static s32 rtl8723_dequeue_writeport(struct adapter *padapter)
>   */
>  s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
>  {
> -	struct xmit_priv *pxmitpriv;
> +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>  	u8 queue_empty, queue_pending;
>  	s32 ret;
>
> -
> -	pxmitpriv = &padapter->xmitpriv;
> -
>  	if (wait_for_completion_interruptible(&pxmitpriv->xmit_comp)) {
>  		netdev_emerg(padapter->pnetdev,
>  			     "%s: down SdioXmitBufSema fail!\n", __func__);
> @@ -242,8 +239,7 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
>  						/* pxmitbuf->priv_data will be NULL, and will crash here */
>  						if (pxmitbuf->len > 0 &&
>  						    pxmitbuf->priv_data) {
> -							struct xmit_frame *pframe;
> -							pframe = (struct xmit_frame *)pxmitbuf->priv_data;
> +							struct xmit_frame *pframe = (struct xmit_frame *)pxmitbuf->priv_data;

I'm not sure that it's worth making this change here given that it makes
the line even longer.

>  							pframe->agg_num = k;
>  							pxmitbuf->agg_num = k;
>  							rtl8723b_update_txdesc(pframe, pframe->buf_addr);
> @@ -326,8 +322,7 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
>  		/*  dump xmit_buf to hw tx fifo */
>  		if (pxmitbuf) {
>  			if (pxmitbuf->len > 0) {
> -				struct xmit_frame *pframe;
> -				pframe = (struct xmit_frame *)pxmitbuf->priv_data;
> +				struct xmit_frame *pframe = (struct xmit_frame *)pxmitbuf->priv_data;

This also makes a long line.

>  				pframe->agg_num = k;
>  				pxmitbuf->agg_num = k;
>  				rtl8723b_update_txdesc(pframe, pframe->buf_addr);
> @@ -357,12 +352,9 @@ static s32 xmit_xmitframes(struct adapter *padapter, struct xmit_priv *pxmitpriv
>   */
>  static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
>  {
> -	struct xmit_priv *pxmitpriv;
> +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>  	s32 ret;
>
> -
> -	pxmitpriv = &padapter->xmitpriv;
> -
>  	if (wait_for_completion_interruptible(&pxmitpriv->SdioXmitStart)) {
>  		netdev_emerg(padapter->pnetdev, "%s: SdioXmitStart fail!\n",
>  			     __func__);
> @@ -408,13 +400,9 @@ static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
>
>  int rtl8723bs_xmit_thread(void *context)
>  {
> -	s32 ret;
> -	struct adapter *padapter;
> -	struct xmit_priv *pxmitpriv;
> -
> -	ret = _SUCCESS;
> -	padapter = context;
> -	pxmitpriv = &padapter->xmitpriv;
> +	s32 ret = _SUCCESS;
> +	struct adapter *padapter = context;
> +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>
>  	allow_signal(SIGTERM);
>
> @@ -435,16 +423,13 @@ s32 rtl8723bs_mgnt_xmit(
>  )
>  {
>  	s32 ret = _SUCCESS;
> -	struct pkt_attrib *pattrib;
> -	struct xmit_buf *pxmitbuf;
> +	struct pkt_attrib *pattrib = &pmgntframe->attrib;
> +	struct xmit_buf *pxmitbuf = pmgntframe->pxmitbuf;
>  	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>  	struct dvobj_priv *pdvobjpriv = adapter_to_dvobj(padapter);
>  	u8 *pframe = (u8 *)(pmgntframe->buf_addr) + TXDESC_OFFSET;
>  	u8 txdesc_size = TXDESC_SIZE;
>
> -	pattrib = &pmgntframe->attrib;
> -	pxmitbuf = pmgntframe->pxmitbuf;
> -
>  	rtl8723b_update_txdesc(pmgntframe, pmgntframe->buf_addr);
>
>  	pxmitbuf->len = txdesc_size + pattrib->last_txcmdsz;
> @@ -557,15 +542,13 @@ s32 rtl8723bs_init_xmit_priv(struct adapter *padapter)
>
>  void rtl8723bs_free_xmit_priv(struct adapter *padapter)
>  {
> -	struct xmit_priv *pxmitpriv;
> +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
>  	struct xmit_buf *pxmitbuf;
> -	struct __queue *pqueue;
> +	struct __queue *pqueue = &pxmitpriv->pending_xmitbuf_queue;
>  	struct list_head *plist, *phead;
>  	struct list_head tmplist;
>
>
> -	pxmitpriv = &padapter->xmitpriv;
> -	pqueue = &pxmitpriv->pending_xmitbuf_queue;
>  	phead = get_list_head(pqueue);
>  	INIT_LIST_HEAD(&tmplist);
>
> --
> 2.43.0
>
>
>

Re: [PATCH v2 2/2] staging: rtl8723bs: Initializing variables at declaration

[Erick Karanja]

On Wed, 2025-04-09 at 11:41 -0400, Julia Lawall wrote:
> 
> 
> On Wed, 9 Apr 2025, Erick Karanja wrote:
> 
> > Make the code more readable by moving trivial
> > initializations up with the declarations instead
> > of wasting a line on that.
> > 
> > Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> > ---
> >  .../staging/rtl8723bs/hal/rtl8723bs_xmit.c    | 39 ++++++---------
> > ----
> >  1 file changed, 11 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > index 5dc1c12fe03e..d134d185bfae 100644
> > --- a/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > +++ b/drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c
> > @@ -120,13 +120,10 @@ static s32 rtl8723_dequeue_writeport(struct
> > adapter *padapter)
> >   */
> >  s32 rtl8723bs_xmit_buf_handler(struct adapter *padapter)
> >  {
> > -	struct xmit_priv *pxmitpriv;
> > +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> >  	u8 queue_empty, queue_pending;
> >  	s32 ret;
> > 
> > -
> > -	pxmitpriv = &padapter->xmitpriv;
> > -
> >  	if (wait_for_completion_interruptible(&pxmitpriv-
> > >xmit_comp)) {
> >  		netdev_emerg(padapter->pnetdev,
> >  			     "%s: down SdioXmitBufSema fail!\n",
> > __func__);
> > @@ -242,8 +239,7 @@ static s32 xmit_xmitframes(struct adapter
> > *padapter, struct xmit_priv *pxmitpriv
> >  						/* pxmitbuf-
> > >priv_data will be NULL, and will crash here */
> >  						if (pxmitbuf->len
> > > 0 &&
> >  						    pxmitbuf-
> > >priv_data) {
> > -							struct
> > xmit_frame *pframe;
> > -							pframe =
> > (struct xmit_frame *)pxmitbuf->priv_data;
> > +							struct
> > xmit_frame *pframe = (struct xmit_frame *)pxmitbuf->priv_data;
> 
> I'm not sure that it's worth making this change here given that it
> makes
> the line even longer.
Hello Julia. I will make the necessary changes.
Thank you.
> 
> >  							pframe-
> > >agg_num = k;
> >  							pxmitbuf-
> > >agg_num = k;
> >  							rtl8723b_u
> > pdate_txdesc(pframe, pframe->buf_addr);
> > @@ -326,8 +322,7 @@ static s32 xmit_xmitframes(struct adapter
> > *padapter, struct xmit_priv *pxmitpriv
> >  		/*  dump xmit_buf to hw tx fifo */
> >  		if (pxmitbuf) {
> >  			if (pxmitbuf->len > 0) {
> > -				struct xmit_frame *pframe;
> > -				pframe = (struct xmit_frame
> > *)pxmitbuf->priv_data;
> > +				struct xmit_frame *pframe =
> > (struct xmit_frame *)pxmitbuf->priv_data;
> 
> This also makes a long line.
> 
> >  				pframe->agg_num = k;
> >  				pxmitbuf->agg_num = k;
> >  				rtl8723b_update_txdesc(pframe,
> > pframe->buf_addr);
> > @@ -357,12 +352,9 @@ static s32 xmit_xmitframes(struct adapter
> > *padapter, struct xmit_priv *pxmitpriv
> >   */
> >  static s32 rtl8723bs_xmit_handler(struct adapter *padapter)
> >  {
> > -	struct xmit_priv *pxmitpriv;
> > +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> >  	s32 ret;
> > 
> > -
> > -	pxmitpriv = &padapter->xmitpriv;
> > -
> >  	if (wait_for_completion_interruptible(&pxmitpriv-
> > >SdioXmitStart)) {
> >  		netdev_emerg(padapter->pnetdev, "%s: SdioXmitStart
> > fail!\n",
> >  			     __func__);
> > @@ -408,13 +400,9 @@ static s32 rtl8723bs_xmit_handler(struct
> > adapter *padapter)
> > 
> >  int rtl8723bs_xmit_thread(void *context)
> >  {
> > -	s32 ret;
> > -	struct adapter *padapter;
> > -	struct xmit_priv *pxmitpriv;
> > -
> > -	ret = _SUCCESS;
> > -	padapter = context;
> > -	pxmitpriv = &padapter->xmitpriv;
> > +	s32 ret = _SUCCESS;
> > +	struct adapter *padapter = context;
> > +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> > 
> >  	allow_signal(SIGTERM);
> > 
> > @@ -435,16 +423,13 @@ s32 rtl8723bs_mgnt_xmit(
> >  )
> >  {
> >  	s32 ret = _SUCCESS;
> > -	struct pkt_attrib *pattrib;
> > -	struct xmit_buf *pxmitbuf;
> > +	struct pkt_attrib *pattrib = &pmgntframe->attrib;
> > +	struct xmit_buf *pxmitbuf = pmgntframe->pxmitbuf;
> >  	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> >  	struct dvobj_priv *pdvobjpriv =
> > adapter_to_dvobj(padapter);
> >  	u8 *pframe = (u8 *)(pmgntframe->buf_addr) + TXDESC_OFFSET;
> >  	u8 txdesc_size = TXDESC_SIZE;
> > 
> > -	pattrib = &pmgntframe->attrib;
> > -	pxmitbuf = pmgntframe->pxmitbuf;
> > -
> >  	rtl8723b_update_txdesc(pmgntframe, pmgntframe->buf_addr);
> > 
> >  	pxmitbuf->len = txdesc_size + pattrib->last_txcmdsz;
> > @@ -557,15 +542,13 @@ s32 rtl8723bs_init_xmit_priv(struct adapter
> > *padapter)
> > 
> >  void rtl8723bs_free_xmit_priv(struct adapter *padapter)
> >  {
> > -	struct xmit_priv *pxmitpriv;
> > +	struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
> >  	struct xmit_buf *pxmitbuf;
> > -	struct __queue *pqueue;
> > +	struct __queue *pqueue = &pxmitpriv-
> > >pending_xmitbuf_queue;
> >  	struct list_head *plist, *phead;
> >  	struct list_head tmplist;
> > 
> > 
> > -	pxmitpriv = &padapter->xmitpriv;
> > -	pqueue = &pxmitpriv->pending_xmitbuf_queue;
> >  	phead = get_list_head(pqueue);
> >  	INIT_LIST_HEAD(&tmplist);
> > 
> > --
> > 2.43.0
> > 
> > 
> >