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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150131161415.GD6456@mwanda>
Date:	Sat, 31 Jan 2015 19:14:15 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	navin patidar <navin.patidar@...il.com>,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8188eu: hal: phy: Removed variables that is
 never used

> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/