Re: [PATCH v4 02/16] Report bugs consistently
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 25, 2016, 22:36 UTC
- Message-ID
- <xmqqlh0p2vmn.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20160725221700.GB14131@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 30 quoted lines
> 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 ;-)