netdev - Re: [net-next v2 03/11] 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:	Fri, 3 Jan 2014 18:31:38 +0000
From:	"Rustad, Mark D" <mark.d.rustad@...el.com>
To:	Joe Perches <joe@...ches.com>
CC:	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	David Miller <davem@...emloft.net>,
	"<netdev@...r.kernel.org>" <netdev@...r.kernel.org>,
	"gospo@...hat.com" <gospo@...hat.com>,
	"sassmann@...hat.com" <sassmann@...hat.com>
Subject: Re: [net-next v2 03/11] ixgbe: Use static inlines instead of macros

On Jan 2, 2014, at 9:28 PM, Joe Perches <joe@...ches.com> wrote:

> On Thu, 2014-01-02 at 21:18 -0800, Jeff Kirsher wrote:
>> From: Mark Rustad <mark.d.rustad@...el.com>
> []
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h
> []
>> @@ -124,22 +124,31 @@ s32 ixgbe_reset_pipeline_82599(struct ixgbe_hw *hw);
>> #ifndef writeq
>> #define writeq(val, addr) writel((u32) (val), addr); \
>>     writel((u32) (val >> 32), (addr + 4));
>> #endif
> 
> This is unchanged, but it would be nicer with a do {} while.

Yes, it is definitely a trap for a macro to directly generate two statements like this. The #ifdef check seemed a little dubious to me when I first saw it, but at the time I chose to stick to what I had to do and then forgot to revisit this.

> #ifndef writeq
> #define writeq(val, addr)				\
> do {							\
> 	writel((u32)(val), addr);			\
> 	writel((u32)((val) >> 32), (addr + 4));		\
> } while (0)

Now that I have looked into it, I only fear becoming a "mucking foron" by touching it at all! :-) (grep the alpha arch code) Lacking an ARCH_HAS macro for detecting writeq, it does appear that this check is currently the way to do it, though it probably makes no sense whatsoever to compile this driver for any architecture that is not defining writeq. I wonder how many of these sequences were added in support of randconfig?

> Even then, this could be nicer as an inline too.

It does appear that the other places checking for writeq are being implemented with static inlines, so that should be the preferred method here as well.

-- 
Mark Rustad, Networking Division, Intel Corporation

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