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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Fri, May 12, 2017 at 11:28:50PM -0400, Jeff King wrote:
>
>> +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.