Re: [PATCH kvmtool 5/6] util: Allow die_perror() to take a variable list of argument

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Will,

On Mon, May 18, 2026 at 04:06:29PM +0100, Will Deacon wrote:
> On Mon, May 18, 2026 at 11:12:56AM +0100, Alexandru Elisei wrote:
> > On Sun, May 17, 2026 at 09:44:30AM +0100, Will Deacon wrote:
> > > On Mon, Mar 23, 2026 at 03:02:20PM +0000, Alexandru Elisei wrote:
> > > > @@ -98,11 +99,23 @@ void __pr_debug(const char *debug, ...)
> > > >  	va_end(params);
> > > >  }
> > > >  
> > > > -void die_perror(const char *s)
> > > > +void die_perror(const char *fmt, ...)
> > > >  {
> > > > +	char buf[1024];
> > > > +	va_list params;
> > > >  	int e = errno;
> > > > +	int ret;
> > > > +
> > > > +	va_start(params, fmt);
> > > > +	ret = vsnprintf(buf, sizeof(buf), fmt, params);
> > > > +	if (ret < 0) {
> > > > +		e = errno;
> > > > +		strncpy(buf, "vsnprintf", ARRAY_SIZE(buf) - 1);
> > > 
> > > That looks really confusing to me as we're effectively hiding the real
> > > error that caused us to get here just because we couldn't pretty-print
> > > the error string! In the unlikely case that vsnprintf() fails, I think
> > > we should preserve the original errno and either print a string to say
> > > that the formatting failed _or_ pretty-print the original error with
> > > strerror().
> > 
> > You have a good point.
> > 
> > I'm not exactly sure what you mean by pretty-printing the original error.
> > Do you mean that we should print the message passed to die_perror()? I'm
> > don't know how I can do that reliably. I could use vprintf(), but if
> > vsnprintf() failed, wouldn't that make it likely that vprintf() will also
> > fail?
> > 
> > How about this:
> > 
> > diff --git a/util/util.c b/util/util.c
> > index 0f116e0ac459..4e3704f781e8 100644
> > --- a/util/util.c
> > +++ b/util/util.c
> > @@ -109,8 +109,9 @@ void die_perror(const char *fmt, ...)
> >         va_start(params, fmt);
> >         ret = vsnprintf(buf, sizeof(buf), fmt, params);
> >         if (ret < 0) {
> > -               e = errno;
> > -               strncpy(buf, "vsnprintf", ARRAY_SIZE(buf) - 1);
> > +               fprintf(stderr, "vsnprintf failed with error %d (%s)\n",
> > +                       errno, strerror(errno));
> > +               strncpy(buf, "Error", ARRAY_SIZE(buf) - 1);
> >         }
> >         va_end(params);
> > 
> > but we're still losing the original message.
> 
> Sorry, I think my suggestion to use strerror() was unhelpful. Maybe just
> do something like:
> 
> 	ret = vsnprintf(buf, sizeof(buf), fmt, params);
> 	if (ret < 0) {
> 		perror("vsnprintf");
> 		buf[0] = '\0';
> 	}
> 
> 	...
> 
> 	errno = e;
> 	perror(buf);
> 

Gotcha.

Thanks,
Alex