Hi Denis,
Denis Vlasenko wrote:
> On Tuesday 11 April 2006 12:49, Ingo Oeser wrote:
> > #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
> > static inline has_vlan_group(...) {
> > /* get VLAN group */
> > }
> > #else
> > static inline has_vlan_group(...) {return 0;}
> > #endif
> >
> > With this and similiar changes in the drivers,
> > your patch might be less intrusive and thus more acceptable to maintainers.
> >
> > Just let the compiler remove the extra code with constant folding and dead
> > code elemination. The result will be even cleaner code, I think.
> >
> > What do you think?
I thought more of introducing functions, which just fold away
all the "if ()" blocks in normal code paths,
which you wrapped into "#if" here.
I don't think people have problems, if you #ifdef out complete functions,
linear setup code or structure members. People seem to have more problems
with #ifdef in control flow code, because there the condition is nothing else
but
a compile time constant (the CONFIG_FOO value) which should be expressed as
such.
Instead of
#ifdef CONFIG_FOO
if (condition) {
}
#endif
just do
#ifdef CONFIG_FOO
#define foo 1
#else
#define foo 0
#endif
if (foo && condition) {
}
Just do nothing or return a compile time constant in those inlines.
>
> Addresses of these functions are stored into netdevice members:
>
> @@ -2549,8 +2559,10 @@ typhoon_init_one(struct pci_dev *pdev, c
> dev->watchdog_timeo = TX_TIMEOUT;
> dev->get_stats = typhoon_get_stats;
> dev->set_mac_address = typhoon_set_mac_address;
> +#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
> dev->vlan_rx_register = typhoon_vlan_rx_register;
> dev->vlan_rx_kill_vid = typhoon_vlan_rx_kill_vid;
> +#endif
>
> Even empty inline would not be "optimized out to nothing" here.
No problem, just optimize out the assignment itself:
#if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
static inline void typhoon_setup_vlan_hooks(struct netdev *dev) {
dev->vlan_rx_register = typhoon_vlan_rx_register;
dev->vlan_rx_kill_vid = typhoon_vlan_rx_kill_vid;
}
#else
static inline void typhoon_setup_vlan_hooks(struct netdev *dev) { (void)dev; }
#endif
The whole linux/if_vlan.h stuff misses this style of code.
There is simply no fallback code for CONFIG_VLAN_8021Q not being defined.
Regards
Ingo Oeser
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
