On Fri, Apr 05, 2024 at 08:59:52PM +0200, René Scharfe wrote:
> vreportf(), which is used e.g. by die() and warning() by default, calls
> vsnprintf(3) to format the message to report. If that call fails, it
> only prints the prefix, e.g. "fatal: " or "warning: ". This at least
> informs users that they were supposed to get a message and reveals its
> severity, but leaves them wondering what it may have been about.
>
> Here's an example where vreportf() tries to print a message with a 2GB
> string, which is too much for vsnprintf(3):
>
> $ perl -le 'print "create refs/heads/", "a"x2**31' | git update-ref --stdin
> fatal:
>
> At least report the formatting error along with the offending message
> (unformatted) to indicate why that message is empty. Use fprintf(3)
> instead of error() to get the message out directly and avoid recursing
> back into vreportf().
>
> With this patch we get:
>
> $ perl -le 'print "create refs/heads/", "a"x2**31' | git update-ref --stdin
> error: unable to format message: invalid ref format: %s
> fatal:
>
> ... which allows users to at least get an idea of what went wrong.
Thanks, I think this is a good change and you've nicely summarized the
situation above. And the patch itself:
> diff --git a/usage.c b/usage.c
> index 09f0ed509b..7a2f7805f5 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -19,8 +19,11 @@ static void vreportf(const char *prefix, const char *err, va_list params)
> }
> memcpy(msg, prefix, prefix_len);
> p = msg + prefix_len;
> - if (vsnprintf(p, pend - p, err, params) < 0)
> + if (vsnprintf(p, pend - p, err, params) < 0) {
> + fprintf(stderr, _("error: unable to format message: %s\n"),
> + err);
> *p = '\0'; /* vsnprintf() failed, clip at prefix */
> + }
is nice and simply, and shouldn't have any unexpected side effects.
-Peff
