On Wed, 2008-04-02 at 08:30 -0600, Matthew Wilcox wrote:
> On Wed, Apr 02, 2008 at 09:15:53AM -0500, James Bottomley wrote:
> > On Tue, 2008-04-01 at 23:32 -0700, Greg KH wrote:
> > > BUG_ON() should not do anything in the macro except test for a value, no
> > > function calling. I think checkpatch.pl checks for this...
> >
> > Well, we can agree to differ on this. The camp that wants no side
> > effects for BUG_ON() does so in case they want to define it to be a nop.
>
> That's one argument, but to me, the most important thing is that reading
> the content of BUG_ON is unnecessary for understanding the function.
>
> > OK ... your subsystem tree your call, I suppose. How about the
> > attached.
>
> > -static inline int transport_container_unregister(struct transport_container *tc)
> > +static inline void transport_container_unregister(struct transport_container *tc)
> > {
> > - return attribute_container_unregister(&tc->ac);
> > + int err = attribute_container_unregister(&tc->ac);
> > + BUG_ON(err);
> > }
>
> What's wrong with:
>
> if (attribute_container_unregister(&tc->ac))
> BUG();
You've lost the unlikely designation which is one of the main reasons
for using BUG_ON(). Most people who write in this form also forget it
leading to a heap of suboptimal jump prediction code in the kernel and
another reason not to encourage it.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
