Re: [PATCH] usage: report vsnprintf(3) failure
- From
Jeff King <peff@peff.net>
- Date
- Apr 5, 2024, 19:20 UTC
- Message-ID
- <20240405192024.GB2561807@coredump.intra.peff.net>
- In-Reply-To
- <3da13298-b6a6-4391-b8e8-5dae9a28b860@web.de>
On Fri, Apr 05, 2024 at 08:59:52PM +0200, René Scharfe wrote:
Show 24 quoted lines
> 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:
Show 14 quoted lines
> 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