netdev - Re: [PATCH] - e1000_ethtool.c - convert macros to functions

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:	Wed, 31 Oct 2007 16:15:56 -0700
From:	Joe Perches <joe@...ches.com>
To:	"Kok, Auke" <auke-jan.h.kok@...el.com>
Cc:	netdev <netdev@...r.kernel.org>, e1000-devel@...ts.sourceforge.net,
	Jeff Garzik <jgarzik@...ox.com>
Subject: Re: [PATCH] - e1000_ethtool.c - convert macros to functions

On Wed, 2007-10-31 at 14:30 -0700, Kok, Auke wrote:
> Joe Perches wrote:
> that's not a bad idea, however see below:
> can't we keep the macro here (and just make it call the function instead of
> expanding). the resulting code is much more lenghty and contains all these logic
> traps that the previous code didn't have.
> just have the macro expand to `if (reg_pattern_test(...)) return 1)` and you don't
> need to change any of the calling lines.

You could define something like:

#define REG_PATTERN_TEST(reg, mask, write) \
	if (reg_pattern_test(adapter, data, \
			     E1000_REG(&adapter->hw, reg), \
			     mask, write)) \
		return 1;

But isn't the macro with an embedded return a bit too obfuscating?

> > +#define E1000_READ_REG_ARRAY(a, reg, offset)		\
> > +	(readl((a)->hw_addr +				\
> > +	       (((a)->mac_type >= e1000_82543)		\
> > +		? E1000_##reg : E1000_82542_##reg) +	\
> > +	       ((offset) << 2)))
> 
> did you have to change these macro's ?

No.  Your choice to keep/remove.
I did want to use the E1000_REG or a new E1000_REG_ADDR macro.

> also, I'm a bit inclined to prefer a patch for e1000e for now as we're about to
> move the pci-express hardware over, but we can certainly merge something like this
> in e1000 after the move as well.

Simple enough.

When is e1000e scheduled to be part of defconfig?

cheers,  Joe

-
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