Re: [RFC PATCH 0/5] Localise error headers

From: Jeff King <peff@peff.net>
Date: Jan 21, 2017, 14:19 UTC
Message-ID: <20170121141942.lc4pw3ck3nzxbozv@sigill.intra.peff.net>
In-Reply-To: <CACsJy8BRdd4f7f7+7XyN8jd69GS6tnkEGhw05F=7c96eViRVQg@mail.gmail.com>

On Fri, Jan 20, 2017 at 08:08:43PM +0700, Duy Nguyen wrote:

Show 8 quoted lines

> > In addition, "BUG: " is relatively recent introduction to our
> > codebase.  Perhaps having a separate BUG(<string>) function help the
> > distinction further?
> 
> I was going to write the same thing. On top of that I wonder if have
> enough "if (something) die("BUG:")" to justify stealing BUG_ON() from
> kernel (better than assert since the condition will always be
> evaluated).

I don't mind BUG_ON() as an improvement over assert(). However, one of the things I really like about our current die("BUG") is that you actually write a human-readable message. Quite often that serves as a comment to the reader of the code as to what is going on, or you can give additional context in the error message (e.g., which sha1 triggered the bug).

Still, there are many cases where there's not much to say in the message. Doing:

  if (!foo)
	BUG("foo is NULL in this function");

is just wasting everybody's time. Something like:

  #define BUG_ON(code) do {
	if (code)
		BUG("%s", #code);
  while(0)

is probably fine. One complication is that BUG() would ideally show the file/line number. That has to happen in a macro, but we don't assume variadic macros work everywhere, which we need for the printf-like behavior. So either we have to loosen that restriction[1], or we end up with two "tiers": a BUG(fmt, ...) for human-readable messages, and a more boring version for BUG_ON() which just prints "file:line: BUG: <code>".

-Peff

[1] I think avoiding variadic macros was reasonable in 2005; C99 was
    only 6 years old then. Now it's turning 18. Maybe it's time?