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

[Jeff King <peff@peff.net>]

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

> > 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?