linux-kernel - Re: [PATCH] e1000: remove unused variables

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] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 28 Nov 2014 15:28:55 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>,
	Hisashi T Fujinaka <htodd@...fifty.com>
CC:	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	Bruce Allan <bruce.w.allan@...el.com>,
	Carolyn Wyborny <carolyn.wyborny@...el.com>,
	Don Skidmore <donald.c.skidmore@...el.com>,
	Greg Rose <gregory.v.rose@...el.com>,
	Matthew Vick <matthew.vick@...el.com>,
	John Ronciak <john.ronciak@...el.com>,
	Mitch Williams <mitch.a.williams@...el.com>,
	Linux NICS <linux.nics@...el.com>,
	e1000-devel@...ts.sourceforge.net, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] e1000: remove unused variables

Le 27/11/2014 05:07, Sudip Mukherjee a écrit :
> On Wed, Nov 26, 2014 at 09:59:28PM -0800, Hisashi T Fujinaka wrote:
>> I'm pretty sure those double reads are there for a reason, so most of
>> this I'm going to have to check on Monday. We have a long holiday
>> weekend here in the US.
> 
> if the double reads are there for some reason, can you please let me know what that reason might be..

Could be latching, especially in the context of reading from Ethernet
PHYs, some registers are latched, so you may have to do a double read to
ensure the value you get is consistent.

Also, if you do a read that is not stored in any return value, the
compiler is now free to remove that actual read, and that may have other
side effects for registers which are e.g: read to clear, or any of the like.

> 
>>
>> I'm not sure why you're bothering with an old driver like this, but if
>> you haven't actually tried this on all the hardware it pertains to, I'm
>> going want to NAK this.
> 
> no it has not been tested on hardware.  :(
> 
> i am still in the learning process, NAK is also part of learning.
> 
> infact there is another part of the code, which, theoretically, will never get executed. but i didnot dare to send that removal patch without testing on the hardware.
> 
> thanks
> sudip
> 
>>
>> I should do this from my todd.fujinaka@...el.com account but it's 10PM
>> on the first day of a long holiday weekend.
>>
>> On Thu, 27 Nov 2014, Sudip Mukherjee wrote:
>>
>>> these variables were only being assigned some values, but were never
>>> used.
>>>
>>> Signed-off-by: Sudip Mukherjee <sudip@...torindia.org>
>>> ---
>>> drivers/net/ethernet/intel/e1000/e1000_hw.c   | 142 ++++++++++++--------------
> <snip>
>>> 			case SPEED_100:
>>> -				txb2b = false;
>>> 				/* maybe add some timeout factor ? */
>>> 				break;
>>> 			}
>>>
>>
>> -- 
>> Hisashi T Fujinaka - htodd@...fifty.com
>> BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
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/