Re: [PATCH 1/3] usage.c: add BUG() function

Jeff King [off-list ref] writes:

On Fri, May 12, 2017 at 11:28:50PM -0400, Jeff King wrote:

quoted

+static NORETURN void BUG_vfl(const char *file, int line, const char *fmt, va_list params)
+{
+	char prefix[256];
+
+	/* truncation via snprintf is OK here */
+	if (file)
+		snprintf(prefix, sizeof(prefix), "BUG: %s:%d: ", file, line);
+	else
+		snprintf(prefix, sizeof(prefix), "BUG: ");
+
+	vreportf(prefix, fmt, params);
+	abort();
+}

I used vreportf() here to match die(). But the two things that function
does are:

  1. Respect error_handle. But after bw/forking-and-threading is merged,
     nobody will ever set error_handle (and I just sent a patch to drop
     it entirely).

  2. Quotes non-printable characters. I don't know how useful this is.
     Most of the assertion messages are pretty vanilla (because anything
     that relies on user input probably should be a regular die, not an
     assertion failure). But a few of them do actually print arbitrary
     strings in a reasonable way (e.g., a BUG() which is handling user
     string which was supposed to be vetted by an earlier function is
     still a reasonable assertion, but it's useful to show the string).

So an alternative would be just:

  fprintf(stderr, "BUG: ");
  if (file)
	fprintf(stderr, "%s:%d ", file, line);
  vfprintf(stderr, fmt, params);
  fputc('\n', stderr);

which is perhaps a bit simpler (not much in lines of code, but there's
no extra buffer to reason about). But given the discussion in (2) above,
it's probably worth continuing to use vreportf.

Yeah, that does sound sensible.  Thanks.