On Thu, Jun 08, 2006 at 10:23:56AM -0700, Mitch Williams wrote:
> On Wed, 2006-06-07 at 11:54 -0700, John W. Linville wrote:
>
> > Pedantic objection, but I think this would read easier w/o the extra
> > newline before disable_irq.
>
> Heh. I prefer to have a newline between declarations and code. The
Normally I would agree. But in this case, I find the distraction of
the random newline after the #else to be more compelling.
> real problem is the position of the #ifdef -- that's what makes it
> difficult to read. The other solution would be
> {
> struct e1000_adapter *adapter = netdev_priv(netdev);
> #ifdef CONFIG_E1000_NAPI
> int budget = 0;
> #endif
>
> disable_irq(adapter->pdev->irq);
>
> #ifdef CONFIG_E1000_NAPI
> < all that stuff >
> #else
> <rest of the stuff >
> #endif
> }
>
> Which I think is worse to read.
I presume it is the double #ifdef that you find objectionable?
I don't really like it, but at least that idiom is quite common.
Given that the disable_irq appears in both code paths (almost by
necessity), there is a certain appeal to having it outside of the
#ifdef block. That seems more maintainable.
To me, the idiomatic #ifdef placement seems more readable, if for no
other reason than familiarity. I suppose we can agree to disagree.
John
--
John W. Linville
[EMAIL PROTECTED]
-
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
