Jeff King <peff@xxxxxxxx> 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.