netdev - Re: [PATCH net-next 0/15] qlcnic: patches for new adapter

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]

Date:	Fri, 24 Aug 2012 12:56:56 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	sony.chacko@...gic.com
Cc:	netdev@...r.kernel.org, Dept_NX_Linux_NIC_Driver@...gic.com
Subject: Re: [PATCH net-next 0/15] qlcnic: patches for new adapter - Qlogic
 83XX CNA

From: Sony Chacko <sony.chacko@...gic.com>
Date: Thu, 23 Aug 2012 21:07:04 -0400

> From: Sony Chacko <sony.chacko@...gic.com>
> 
> Patch series will restructure the existing 82XX adapter driver to create
> a common driver for Qlogic 82XX and 83XX adapters.
> 
> Please apply it to net-next.

Sorry, this is terrible.

Firstly, I'm not going to let you create arbitrary new sysfs and
debugfs crap for facilities that are already supported by the kernel
via other interfaces.

For example, providing a facility to read and write the PCI BARs
is completely pointless.  Use the PCI config space access APIs for
this if you want to do this from userspace.

The patches are also much more verbose than they need to be.  When you
move an operation to the new hwops, keep the existing function name
but make it an inline function that invokes the hwop.

That way you won't need hundreds of lines in your patch that look
like this:

-	qlcnic_clear_lb_mode(adapter);
+	ahw->hw_ops->clear_loopback(adapter, mode);

Instead you'd have:

static inline void qlcnic_clear_lb_mode(struct qlcnic_adapter *adapter, u8 mode)
{
	struct qlcnic_hardware_context *ahw = &adapter->ahw;

	ahw->hw_ops->clear_loopback(adapter, mode);
}
--
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