[PATCH 1/3] usage: refactor die-recursion checks

[Jeff King <peff@peff.net>]

When any git code calls die(), we chain to a custom
die_routine, which we expect to print a message and exit the
program. To avoid infinite loops, we detect a recursive call
to die() with a simple counter, and break out of the loop by
printing a message and exiting ourselves, without chaining
to the die_routine.

But the user does not get to see the message that would have
been fed to the die_routine, which makes debugging harder.
The user does not know if it was a true infinite loop, or
simply a single re-entrant call, since they cannot compare
the messages. Furthermore, if we are wrong about detecting
the recursion, we have blocked the user from seeing the
original message, which is probably the more useful one.

This patch teaches die() to print the original die message
to stderr before reporting the recursion. The custom
die_routine may or may not have put it the message to
stderr, but this is the best we can do (it is what most
handlers will do anyway, and it is where our recursion error
will go).

While we're at it, let's mark the "recursion detected"
message as a "BUG:", since it should never happen in
practice. And let's factor out the repeated code in die and
die_errno. This loses the information of which function was
called to cause the recursion, but it's important; knowing
the actual message fed to the function (which we now do) is
much more useful, as it can generally pin-point the actual
call-site that caused the recursion.

Signed-off-by: Jeff King <peff@peff.net>
---
This helped me debug the current problem. And factoring it out helps
with patch 3. :)

 usage.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/usage.c b/usage.c
index 40b3de5..c6b7ac5 100644
--- a/usage.c
+++ b/usage.c
@@ -6,8 +6,6 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
-static int dying;
-
 void vreportf(const char *prefix, const char *err, va_list params)
 {
 	char msg[4096];
@@ -80,17 +78,24 @@ void NORETURN die(const char *err, ...)
 	usagef("%s", err);
 }
 
+static void check_die_recursion(const char *fmt, va_list ap)
+{
+	static int dying;
+
+	if (!dying++)
+		return;
+
+	vreportf("fatal: ", fmt, ap);
+	fputs("BUG: recursion detected in die handler\n", stderr);
+	exit(128);
+}
+
 void NORETURN die(const char *err, ...)
 {
 	va_list params;
 
-	if (dying) {
-		fputs("fatal: recursion detected in die handler\n", stderr);
-		exit(128);
-	}
-	dying = 1;
-
 	va_start(params, err);
+	check_die_recursion(err, params);
 	die_routine(err, params);
 	va_end(params);
 }
@@ -102,13 +107,6 @@ void NORETURN die_errno(const char *fmt, ...)
 	char str_error[256], *err;
 	int i, j;
 
-	if (dying) {
-		fputs("fatal: recursion detected in die_errno handler\n",
-			stderr);
-		exit(128);
-	}
-	dying = 1;
-
 	err = strerror(errno);
 	for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
 		if ((str_error[j++] = err[i++]) != '%')
@@ -126,6 +124,7 @@ void NORETURN die_errno(const char *fmt, ...)
 	snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, str_error);
 
 	va_start(params, fmt);
+	check_die_recursion(fmt_with_err, params);
 	die_routine(fmt_with_err, params);
 	va_end(params);
 }
-- 
1.8.2.8.g44e4c28