From b4990734066b6333f4a6b68718afafa1ecaa928d Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 17 May 2016 09:06:32 +0200 Subject: [PATCH] dprintf_formatf: fix (false?) Coverity warning CID 1024412: Memory - illegal accesses (OVERRUN). Claimed to happen when we run over 'workend' but the condition says <= workend and for all I can see it should be safe. Compensating for the warning by adding a byte margin in the buffer. Also, removed the extra brace level indentation in the code and made it so that 'workend' is only assigned once within the function. --- lib/mprintf.c | 113 ++++++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 55 deletions(-) diff --git a/lib/mprintf.c b/lib/mprintf.c index 022012c8c..73f854bcb 100644 --- a/lib/mprintf.c +++ b/lib/mprintf.c @@ -581,6 +581,11 @@ static int dprintf_formatf( va_stack_t *p; + /* 'workend' points to the final buffer byte position, but with an extra + byte as margin to avoid the (false?) warning Coverity gives us + otherwise */ + char *workend = &work[sizeof(work) - 2]; + /* Do the actual %-code parsing */ dprintf_Pass1(format, vto, endpos, ap_save); @@ -610,6 +615,8 @@ static int dprintf_formatf( /* Used to convert negative in positive. */ mp_intmax_t signed_num; + char *w; + if(*f != '%') { /* This isn't a format spec, so write everything out until the next one OR end of string is reached. */ @@ -730,72 +737,68 @@ static int dprintf_formatf( number: /* Number of base BASE. */ - { - char *workend = &work[sizeof(work) - 1]; - char *w; - /* Supply a default precision if none was given. */ - if(prec == -1) - prec = 1; + /* Supply a default precision if none was given. */ + if(prec == -1) + prec = 1; - /* Put the number in WORK. */ - w = workend; - while(num > 0) { - *w-- = digits[num % base]; - num /= base; - } - width -= (long)(workend - w); - prec -= (long)(workend - w); + /* Put the number in WORK. */ + w = workend; + while(num > 0) { + *w-- = digits[num % base]; + num /= base; + } + width -= (long)(workend - w); + prec -= (long)(workend - w); - if(is_alt && base == 8 && prec <= 0) { + if(is_alt && base == 8 && prec <= 0) { + *w-- = '0'; + --width; + } + + if(prec > 0) { + width -= prec; + while(prec-- > 0) *w-- = '0'; - --width; - } + } - if(prec > 0) { - width -= prec; - while(prec-- > 0) - *w-- = '0'; - } + if(is_alt && base == 16) + width -= 2; - if(is_alt && base == 16) - width -= 2; + if(is_neg || (p->flags & FLAGS_SHOWSIGN) || (p->flags & FLAGS_SPACE)) + --width; - if(is_neg || (p->flags & FLAGS_SHOWSIGN) || (p->flags & FLAGS_SPACE)) - --width; - - if(!(p->flags & FLAGS_LEFT) && !(p->flags & FLAGS_PAD_NIL)) - while(width-- > 0) - OUTCHAR(' '); - - if(is_neg) - OUTCHAR('-'); - else if(p->flags & FLAGS_SHOWSIGN) - OUTCHAR('+'); - else if(p->flags & FLAGS_SPACE) + if(!(p->flags & FLAGS_LEFT) && !(p->flags & FLAGS_PAD_NIL)) + while(width-- > 0) OUTCHAR(' '); - if(is_alt && base == 16) { - OUTCHAR('0'); - if(p->flags & FLAGS_UPPER) - OUTCHAR('X'); - else - OUTCHAR('x'); - } + if(is_neg) + OUTCHAR('-'); + else if(p->flags & FLAGS_SHOWSIGN) + OUTCHAR('+'); + else if(p->flags & FLAGS_SPACE) + OUTCHAR(' '); - if(!(p->flags & FLAGS_LEFT) && (p->flags & FLAGS_PAD_NIL)) - while(width-- > 0) - OUTCHAR('0'); - - /* Write the number. */ - while(++w <= workend) { - OUTCHAR(*w); - } - - if(p->flags & FLAGS_LEFT) - while(width-- > 0) - OUTCHAR(' '); + if(is_alt && base == 16) { + OUTCHAR('0'); + if(p->flags & FLAGS_UPPER) + OUTCHAR('X'); + else + OUTCHAR('x'); } + + if(!(p->flags & FLAGS_LEFT) && (p->flags & FLAGS_PAD_NIL)) + while(width-- > 0) + OUTCHAR('0'); + + /* Write the number. */ + while(++w <= workend) { + OUTCHAR(*w); + } + + if(p->flags & FLAGS_LEFT) + while(width-- > 0) + OUTCHAR(' '); break; case FORMAT_STRING: