netdev - Re: [net-next 3/7] ixgbe: Use static inlines instead of macros

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:	Thu, 9 Jan 2014 20:14:51 +0000
From:	"Rustad, Mark D" <mark.d.rustad@...el.com>
To:	David Miller <davem@...emloft.net>
CC:	Scott Feldman <sfeldma@...ulusnetworks.com>,
	"Brown, Aaron F" <aaron.f.brown@...el.com>,
	Netdev <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: Re: [net-next 3/7] ixgbe: Use static inlines instead of macros

On Jan 9, 2014, at 11:39 AM, David Miller <davem@...emloft.net> wrote:

> From: "Rustad, Mark D" <mark.d.rustad@...el.com>
> Date: Thu, 9 Jan 2014 17:34:18 +0000
> 
>> On Jan 8, 2014, at 12:47 AM, Scott Feldman <sfeldma@...ulusnetworks.com> wrote:
>> 
>>> 
>>> On Jan 7, 2014, at 11:40 PM, Aaron Brown <aaron.f.brown@...el.com> wrote:
>>> 
>>>> From: Mark Rustad <mark.d.rustad@...el.com>
>>>> 
>>>> -#define IXGBE_WRITE_REG(a, reg, value) writel((value), ((a)->hw_addr + (reg)))
>>>> +static inline void IXGBE_WRITE_REG(struct ixgbe_hw *hw, u32 reg, u32 value)
>>> 
>>> Bummer, now you have a all-caps func name.
>> 
>> Agreed, but this is actually a fairly common condition among drivers that used to use macros. It isn't perfect, but at least it is moving in the right direction. I'd rather leave the case change for a later patch series that does only that or has some reason to touch all of the register access sites.
>> 
>> At least the new accessor I introduced is lower case. :-)
> 
> Please address this feedback, all caps function names are really not
> appropriate.

I really don't think it is a good idea to do that as part of this patch series. It makes this patch series a pretty solid barrier to any other patches going into this driver because it would change the name of all the register accessors.

This makes me want to deal with that as a separate issue, since there would be no functional reason to drop such a patch and it can be planned into a workflow.

Obviously I could do it here, but I *really* think it is procedurally a really bad idea to change the case as part of a functional change. I thought I was doing a favor my at least making them inlines, but prehaps not.

Anyone want to take on changing the upper case static inlines in mcf8390, 7990, benet, ns83820, s2io, vxge, iwlwifi, ath9k, wil6210, mwifiex, and rtlwifi? And those are just under drivers/net.

-- 
Mark Rustad, Networking Division, Intel Corporation

Download attachment "signature.asc" of type "application/pgp-signature" (842 bytes)