Messages in this thread | | | Date | Wed, 21 Jan 2004 12:39:38 -0600 | From | Matt Mackall <> | Subject | Re: More cleanups for swsusp |
| |
On Tue, Jan 20, 2004 at 09:30:37PM -0800, Andrew Morton wrote: > Rusty Russell <rusty@rustcorp.com.au> wrote: > > > > In message <20040120225219.GA19190@elf.ucw.cz> you write: > > > - if (fill_suspend_header(&cur->sh)) > > > - panic("\nOut of memory while writing header"); > > > + BUG_ON (fill_suspend_header(&cur->sh)); > > > > ... > > 3) BUG_ON(complex condition expression) is much less clear than: > > > > if (complex condition expression) > > BUG();
Disagree. All BUG_ON() stuff should read like:
/* check that impossible stuff didn't happen, move along, nothing to see */ BUG_ON(...);
Which is fine and good until the condition is actually doing more than just sanity checking.
> Worse. If some smarty goes and makes BUG_ON a no-op (for space reasons), > it will break software suspend. We should ensure that the expression which > is supplied to BUG_ON() never has side-effects for this reason.
While I generally agree that "assertions" shouldn't have side-effects, a slightly smarter smarty would make sure that BUG_ON evaluated its condition. I have this in -tiny:
+#ifndef CONFIG_BUG +#define BUG() +#define WARN_ON(condition) do { if (condition) ; } while(0) +#define BUG_ON(condition) do { if (condition) ; } while(0) +#define PAGE_BUG(page) +#else
-- Matt Mackall : http://www.selenic.com : Linux development and consulting - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |