[RFC PATCH 20/21] usage API: make the "{usage,fatal,error,warning,BUG}: " translatable
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 15, 2021, 22:18 UTC
- Message-ID
- <RFC-patch-20.21-69426ddb992-20211115T220831Z-avarab@gmail.com>
- In-Reply-To
- <RFC-cover-00.21-00000000000-20211115T220831Z-avarab@gmail.com>
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 --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);
-- 2.34.0.rc2.809.g11e21d44b24