Re: [PATCH 1/3] usage.c: add BUG() function
- Date: Mon, 15 May 2017 11:28:42 +0900
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH 1/3] usage.c: add BUG() function
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;
>> + /* 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.