Re: [Outreachy kernel] [PATCH v3] Staging: rtl8188eu: Reindent code to reduce line size

[Jes Sorensen <jes.sorensen@gmail.com>]

On 02/23/15 04:44, Vatika Harlalka wrote:
> Reindent code to reduce line size and increase readability.
> 
> Signed-off-by: Vatika Harlalka <vatikaharlalka@gmail.com>
> ---
> Changes in v3: Fixed spelling error in commit message.
> Changes in v2: Changed subject line to imperative form ( "Reindented" to "Reindent" ).
> 
>  drivers/staging/rtl8188eu/hal/bb_cfg.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/hal/bb_cfg.c b/drivers/staging/rtl8188eu/hal/bb_cfg.c
> index e1b5437..5ad080a 100644
> --- a/drivers/staging/rtl8188eu/hal/bb_cfg.c
> +++ b/drivers/staging/rtl8188eu/hal/bb_cfg.c
> @@ -508,7 +508,8 @@ static u32 array_phy_reg_pg_8188e[] = {
>  
>  };
>  
> -static void store_pwrindex_offset(struct adapter *Adapter, u32 regaddr, u32 bitmask, u32 data)
> +static void store_pwrindex_offset(struct adapter *Adapter,
> +				u32 regaddr, u32 bitmask, u32 data)
>  {
>  	struct hal_data_8188e *hal_data = GET_HAL_DATA(Adapter);
>  	 u8 pwrGrpCnt = hal_data->pwrGroupCnt;

It looks like you already started addressing the follow-on issue that I
wanted to raise, but you didn't specify that this patch should go on top
of another patch.

You shouldn't really use u8 for an index, it is better to use int, and
if you pick a shorter name it will help breaking down the lines in the
follow-on code. I would recommend doing something like this:

	int idx = hal_data->pwrGroupCnt;

	if (regaddr == rTxAGC_A_Rate18_06)
		hal_data->MCSTxPowerLevelOriginalOffset[idx][0] = data;
..... do this for all users of hal_data->pwrGroupCnt in this function.

Cheers,
Jes

> @@ -553,7 +554,8 @@ static void store_pwrindex_offset(struct adapter *Adapter, u32 regaddr, u32 bitm
>  	}
>  }
>  
> -static void rtl_addr_delay(struct adapter *adapt, u32 addr, u32 bit_mask, u32 data)
> +static void rtl_addr_delay(struct adapter *adapt,
> +			u32 addr, u32 bit_mask, u32 data)
>  {
>  	if (addr == 0xfe)
>  		msleep(50);
>