Re: [PATCH] usage: report vsnprintf(3) failure

[Jeff King <peff@peff.net>]

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