diff mbox series

[RFC,20/21] usage API: make the "{usage,fatal,error,warning,BUG}: " translatable

Message ID RFC-patch-20.21-69426ddb992-20211115T220831Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series C99: show meaningful <file>:<line> in trace2 via macros | expand

Commit Message

Ævar Arnfjörð Bjarmason Nov. 15, 2021, 10:18 p.m. UTC
In preceding commits the vreportf() function was made static, so we
know it's only being called with a limited set of fixed prefixes. Pass
an enum indicating the kind of usage message we're emitting instead,
which means that we can fold the BUG_vfl_common() functionality
directly into it.

Since we've now got one place were we're emitting these usage messages
we can make them translatable.

We need to be careful with this function to not malloc() anything, as
a failure in say a use of strbuf_vaddf() would call xmalloc(), which
would in turn call die(), but here we're using static strings, either
from libintl or not.

I was on the fence about making the "BUG: " message translatable, but
let's do it for consistency. Someone who doesn't speak a word of
English may not know what "BUG" means, but if it's translated they
might have an easier time knowing that they have to report a bug
upstream. Since we'll always emit the line number it's unlikely that
we're going to be confused by such a report.

As we've moved the BUG_vfl_common() code into vsnprintf() we can do
away with one of the two checks for buffer sizes added in
116d1fa6c69 (vreportf(): avoid relying on stdio buffering, 2019-10-30)
and ac4896f007a (fmt_with_err: add a comment that truncation is OK,
2018-05-18).

I.e. we're being overly paranoid if we define the fixed-size "prefix"
and "msg" buffers, are OK with the former being truncated, and then
effectively check if our 256-byte buffer is larger than our 4096-byte
buffer. I wondered about adding a:

    assert(sizeof(prefix) < sizeof(msg)); /* overly paranoid much? */

But I think that would be overdoing it. Anyone modifying this function
will keep these two buffer sizes in mind, so let's just remove one of
the checks instead.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 usage.c | 68 ++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 23 deletions(-)
diff mbox series

Patch

diff --git a/usage.c b/usage.c
index e6f609fe49a..62313862977 100644
--- a/usage.c
+++ b/usage.c
@@ -6,16 +6,49 @@ 
 #include "git-compat-util.h"
 #include "cache.h"
 
-static void vreportf(const char *prefix, const char *err, va_list params)
+enum usage_kind {
+	USAGE_USAGE,
+	USAGE_DIE,
+	USAGE_ERROR,
+	USAGE_WARNING,
+	USAGE_BUG,
+};
+
+static void vreportf(enum usage_kind kind,
+		     const char *file, int line,
+		     const char *err, va_list params)
 {
+	const char *prefix_i18n;
+	char prefix[256];
 	char msg[4096];
 	char *p, *pend = msg + sizeof(msg);
-	size_t prefix_len = strlen(prefix);
-
-	if (sizeof(msg) <= prefix_len) {
-		fprintf(stderr, "BUG!!! too long a prefix '%s'\n", prefix);
-		abort();
+	size_t prefix_len;
+
+	switch (kind) {
+	case USAGE_USAGE:
+		prefix_i18n =_("usage: ");
+		break;
+	case USAGE_DIE:
+		prefix_i18n =_("fatal: ");
+		break;
+	case USAGE_ERROR:
+		prefix_i18n =_("error: ");
+		break;
+	case USAGE_WARNING:
+		prefix_i18n =_("warning: ");
+		break;
+	case USAGE_BUG:
+		prefix_i18n =_("BUG: ");
+		break;
 	}
+
+	/* truncation via snprintf is OK here */
+	if (kind == USAGE_BUG)
+		snprintf(prefix, sizeof(prefix), "%s%s:%d: ", prefix_i18n, file, line);
+	else
+		snprintf(prefix, sizeof(prefix), "%s", prefix_i18n);
+
+	prefix_len = strlen(prefix);
 	memcpy(msg, prefix, prefix_len);
 	p = msg + prefix_len;
 	if (vsnprintf(p, pend - p, err, params) < 0)
@@ -33,7 +66,7 @@  static void vreportf(const char *prefix, const char *err, va_list params)
 
 static NORETURN void usage_builtin(const char *file, int line, const char *err, va_list params)
 {
-	vreportf("usage: ", err, params);
+	vreportf(USAGE_USAGE, file, line, err, params);
 
 	/*
 	 * When we detect a usage error *before* the command dispatch in
@@ -58,7 +91,7 @@  static NORETURN void usage_builtin(const char *file, int line, const char *err,
 static void die_message_builtin(const char *file, int line, const char *err, va_list params)
 {
 	trace2_cmd_error_va_fl(file, line, err, params);
-	vreportf("fatal: ", err, params);
+	vreportf(USAGE_DIE, file, line, err, params);
 }
 
 /*
@@ -78,14 +111,14 @@  static void error_builtin(const char *file, int line, const char *err, va_list p
 {
 	trace2_cmd_error_va_fl(file, line, err, params);
 
-	vreportf("error: ", err, params);
+	vreportf(USAGE_ERROR, file, line, err, params);
 }
 
 static void warning_builtin(const char *file, int line, const char *warn, va_list params)
 {
 	trace2_cmd_error_va_fl(file, line, warn, params);
 
-	vreportf("warning: ", warn, params);
+	vreportf(USAGE_WARNING, file, line, warn, params);
 }
 
 static int die_is_recursing_builtin(void)
@@ -283,24 +316,13 @@  void warning_errno_fl(const char *file, int line, const char *fmt, ...)
 /* Only set this, ever, from t/helper/, when verifying that bugs are caught. */
 int BUG_exit_code;
 
-static void BUG_vfl_common(const char *file, int line, const char *fmt,
-			   va_list params)
-{
-	char prefix[256];
-
-	/* truncation via snprintf is OK here */
-	snprintf(prefix, sizeof(prefix), "BUG: %s:%d: ", file, line);
-
-	vreportf(prefix, fmt, params);
-}
-
 static NORETURN void BUG_vfl(const char *file, int line, const char *fmt, va_list params)
 {
 	va_list params_copy;
 	static int in_bug;
 
 	va_copy(params_copy, params);
-	BUG_vfl_common(file, line, fmt, params);
+	vreportf(USAGE_BUG, file, line, fmt, params);
 
 	if (in_bug)
 		abort();
@@ -330,7 +352,7 @@  int bug_fl(const char *file, int line, const char *fmt, ...)
 
 	va_copy(cp, ap);
 	va_start(ap, fmt);
-	BUG_vfl_common(file, line, fmt, ap);
+	vreportf(USAGE_BUG, file, line, fmt, ap);
 	va_end(ap);
 	trace2_cmd_error_va_fl(file, line, fmt, cp);