On Thursday 12 July 2007 23:53, Andrew Morton wrote:
> On Thu, 12 Jul 2007 23:42:26 +0200
> Michael Buesch <mb@xxxxxxxxx> wrote:
>
> > > > +#define assert(cond) do { \
> > > > + if (unlikely(!(cond))) { \
> > > > + ssb_dprintk(KERN_ERR PFX "BUG: Assertion failed (%s) " \
> > > > + "at: %s:%d:%s()\n", \
> > > > + #cond, __FILE__, __LINE__, __func__); \
> > > > + } \
> > > > + } while (0)
> > >
> > > Odd. One would normally expect an assert() to terminate execution in some
> > > fashion if it fails. In-kernel that means BUG. But this assert() just
> > > whines and continues.
> >
> > An assertion failure is not a fatal bug, so we continue execution.
> > We do the same in bcm43xx and I really think we mustn't BUG on an
> > assertion failure, as people would already have killed me.
> > Additionally to that, I insert really lots of assert()s into the code.
> > I don't want all these to be WARN_ONs or BUGs, as it would bloat the
> > kernel a lot. In the places where I want runtime checks in nondebug
> > builds, I already use WARN_ON.
>
> Do `man 3 assert'. The reader of your code will expect that an assert()
> will "terminate the program" (or the kernel equivalent) if the assertion
> fails.
>
> So the principle of least surprise tells us "this shouldn't be called
> assert()".
Well, I do know that userspace assert() terminates the program.
But, in the kernel we use BUG() for this.
So let's better rename BUG() to assert() ;)
No just kidding.
IMO the word "assert" is short and to the point what this code
is actually doing. It asserts that a condition is true and
complains otherwise.
Let's make a deal, Andrew.
As I almost always do assert(0), I will remove the assert() macro
and introduce a macro SSB_CAN_NOT_REACH() or something like that
to mark codepaths that can not be reached.
I'll replace the rest of the assert()s that check an actual condition
with WARN_ON.
OK?
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
