Re: [PATCH] staging: rtl8188eu: hal: phy: Removed variables that is never used

[Dan Carpenter]

> I have also removed all the code that thereby serves no purpose.

[ snip ]

> @@ -743,7 +743,7 @@ static u8 phy_path_a_iqk(struct adapter *adapt, bool config_pathb)
>  	reg_eac = phy_query_bb_reg(adapt, rRx_Power_After_IQK_A_2, bMaskDWord);
>  	reg_e94 = phy_query_bb_reg(adapt, rTx_Power_Before_IQK_A, bMaskDWord);
>  	reg_e9c = phy_query_bb_reg(adapt, rTx_Power_After_IQK_A, bMaskDWord);
> -	reg_ea4 = phy_query_bb_reg(adapt, rRx_Power_Before_IQK_A_2, bMaskDWord);
> +	phy_query_bb_reg(adapt, rRx_Power_Before_IQK_A_2, bMaskDWord);
>  
>  	if (!(reg_eac & BIT28) &&
>  	    (((reg_e94 & 0x03FF0000)>>16) != 0x142) &&

You clearly didn't remove *all* the "code that thereby serves no
purpose".

Could we call a cease fire on these and stop sending them until Greg or
someone else weighs in?  It's hard for me to comment on 50 patches that
I don't like.

To re-iterate:

1) Removing part of the line is 90% likely to be wrong.  We can't know
   without knowing the code better, knowing the hardware, or testing.

2) Silencing static checker warnings in the wrong way, negates the
   usefulness of static checkers and makes bad code hard to find.  Some
   of these "unused" variable warnings could easily be bugs so we are
   hiding bugs.

3) From an aesthetic point of view the code was easier to understand
   before.  The line you have deleted was clearly copy and pasted from
   the line before.  But now it looks deliberate and the information we
   need to understand the original intent is hidden in the git log.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/