* David Howells <dhowells@xxxxxxxxxx> wrote: > This facility can be made use of by one of: > > ANNOTATED_BUG(const char *fmt, ...); > ANNOTATED_BUG_ON(bool condition, const char *fmt, ...); Hm, what about WARN_ON()s? WARN_ON() and WARN_ON_ONCE() covers like 99% of the *actual* bugreports we get: we are actively getting rid of BUG()s that do trigger and are asking all new patches to come with WARN_ON()s. BUG()s are generally a poor way of reporting bugs. Another, much bigger issue is the actual syntax: > - BUG_ON(atomic_read(&cookie->usage) <= 0); > + ASSERTCMP(atomic_read(&cookie->usage), >, 0); NAK on that concept on two grounds! 1) BUG_ON() is a well-known pattern. Changing it to the inverted assert() braindamage is going to cause confusion years down the road. Years ago we've settled on using BUG*() and WARN*() assertions to include conditions that check the 'bad' condition'. assert() covers the negated 'good' condition - which works but the two are truly awful when mixed. 2) The '>,0' syntax is ugly. Why don't we simply extend the *existing* primitives with a 'verbose' variant that saves the text string of the macro using the '#param' syntax, intead of modifying the usage sites with a pointless splitting along logical ops? Doing that would also remove the rather pointess ANNOTATED_() prefix, which is like totally uninteresting in the actual usage sites. WARN()s want to be as short, obvious and low-profile as possible. So the whole schizophrenic split between BUG_ON()/WARN_ON() and the assert() world is nonsensical and confusing - we should settle on *one* logical variant to make code reading easier. And i thought in the kernel we already settled on one of these variants and are using it almost exclusively ... So this series does not look good enough to me yet. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
