Re: [PATCH v4 02/16] Report bugs consistently

From: Jeff King <peff@peff.net>
Date: Jul 25, 2016, 22:17 UTC
Message-ID: <20160725221700.GB14131@sigill.intra.peff.net>
In-Reply-To: <xmqqfuqx4cli.fsf@gitster.mtv.corp.google.com>

On Mon, Jul 25, 2016 at 02:44:25PM -0700, Junio C Hamano wrote:

Show 24 quoted lines

> > diff --git a/imap-send.c b/imap-send.c
> > index db0fafe..67d67f8 100644
> > --- a/imap-send.c
> > +++ b/imap-send.c
> > @@ -506,12 +506,12 @@ static char *next_arg(char **s)
> >  
> >  static int nfsnprintf(char *buf, int blen, const char *fmt, ...)
> >  {
> > -	int ret;
> > +	int ret = -1;
> >  	va_list va;
> >  
> >  	va_start(va, fmt);
> >  	if (blen <= 0 || (unsigned)(ret = vsnprintf(buf, blen, fmt, va)) >= (unsigned)blen)
> > -		die("Fatal: buffer too small. Please report a bug.");
> > +		die("BUG: buffer too small (%d < %d)", ret, blen);
> >  	va_end(va);
> >  	return ret;
> >  }
> 
> If "you gave me this size but you need at least this much" is truly
> worth reporting, then this is misleading (ret is shown as -1 but you
> do not even know how much is necessary).  In any case, this should
> be done as a separate step anyway.

Hrm, isn't "ret" going to be the necessary size? According to the standard, it should tell us how many bytes were needed, not "-1" (this is the "your vsnprintf is broken" case handled by the strbuf code).

I do think the numbers are reversed, though. It should be "blen < ret".

-Peff