Jeff King <peff@xxxxxxxx> writes: > On Mon, Jul 25, 2016 at 02:44:25PM -0700, Junio C Hamano wrote: > >> > 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). Yes. If blen <= 0, we do not even do vsnprintf() and that is why Dscho added "int ret = -1" initialization; otherwise his new die() would end up referencing uninitialized ret. > I do think the numbers are reversed, though. It should be "blen < ret". That, too ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html