Re: [PATCH 1/5] usage.c: add a non-fatal bug() function to go with BUG()

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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Any caller to bug() should follow up such calls with BUG_if_bug(),
> which will BUG() out (i.e. abort()) if there were any preceding calls
> to bug(). As the tests and documentation here show we'll catch missing
> BUG_if_bug() invocations in our exit() wrapper.

...

> +- `bug` (lower-case, not `BUG`) is supposed to be used like `BUG` but
> +  prints a "BUG" message instead of calling `abort()`. We then expect
> +  `BUG_if_bug()` to be called to `abort()` if there were any calls to
> +  `bug()`. If there weren't any a call to `BUG_if_bug()` is a NOOP.

OK.  So the expected pattern would be a series of calls to bug(),
each guarded by its own condition, concluded by a call to BUG_if_bug()

	if (condition1)
		bug(...);
	if (condition2)
		bug(...);
	...
	BUG_if_bug();

and when none of the guard conditions fired, BUG_if_bug() will
become a no-op.

> +/* usage.c: if bug() is called we should have a BUG_if_bug() afterwards */
> +extern int bug_called_must_BUG;

I am not sure about the name, but in essense, each call to bug()
ensures that this becomes true, so BUG_if_bug() can use it to see
if it should abort(), right?

>  __attribute__((format (printf, 3, 4))) NORETURN
>  void BUG_fl(const char *file, int line, const char *fmt, ...);
>  #define BUG(...) BUG_fl(__FILE__, __LINE__, __VA_ARGS__)
> +__attribute__((format (printf, 3, 4)))
> +void bug_fl(const char *file, int line, const char *fmt, ...);
> +#define bug(...) bug_fl(__FILE__, __LINE__, __VA_ARGS__)
> +#define BUG_if_bug() do { \
> +	if (bug_called_must_BUG) { \
> +		bug_called_must_BUG = 0; \
> +		BUG_fl(__FILE__, __LINE__, "see bug() output above"); \
> +	} \
> +} while (0)

I have a feeling that "see bug() output above" should come from the
caller of BUG_if_bug().  These bug() calls that are grouped together
must have a shared overall theme, which may want to be stated in
that final message.

Other than these two small points, this does not look too bad ;-)

Thanks.