Paolo Bonzini <pbonz...@redhat.com> writes: > On 03/05/2017 13:38, Markus Armbruster wrote: >> Paolo Bonzini <pbonz...@redhat.com> writes: >> >>> Detected by GCC 7's -Wformat-truncation. snprintf writes at most >>> 2 bytes here including the terminating NUL, so the result is >>> truncated. In addition, the newline at the end is pointless. >>> Fix the buffer size and the format string. >>> --- >>> hw/display/jazz_led.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/display/jazz_led.c b/hw/display/jazz_led.c >>> index b72fdb1717..3c97d56434 100644 >>> --- a/hw/display/jazz_led.c >>> +++ b/hw/display/jazz_led.c >>> @@ -227,13 +227,13 @@ static void jazz_led_invalidate_display(void *opaque) >>> static void jazz_led_text_update(void *opaque, console_ch_t *chardata) >>> { >>> LedState *s = opaque; >>> - char buf[2]; >>> + char buf[3]; >>> >>> dpy_text_cursor(s->con, -1, -1); >>> qemu_console_resize(s->con, 2, 1); >>> >>> /* TODO: draw the segments */ >>> - snprintf(buf, 2, "%02hhx\n", s->segments); >>> + snprintf(buf, 3, "%02hhx", s->segments); >>> console_write_ch(chardata++, ATTR2CHTYPE(buf[0], QEMU_COLOR_BLUE, >>> QEMU_COLOR_BLACK, 1)); >>> console_write_ch(chardata++, ATTR2CHTYPE(buf[1], QEMU_COLOR_BLUE, >> >> Since we're only every interested in the first two characters, the >> truncation is totally harmless. Thus, your patch cleans doesn't really >> "fix bad snprintf", it cleans up an unclean one. Consider rewording the >> commit message accordingly. > > My reading is that buf[1] is always '\0' in the current code: "snprintf > writes at most 2 bytes here including the terminating NUL, so the result > is truncated".
You're right; I forgot that snprintf() always adds a NUL. So this *is* broken: we write NUL instead of the second digit. Mentioning this in the commit message wouldn't hurt. > > Paolo > >> Regardless, >> Reviewed-by: Markus Armbruster <arm...@redhat.com> >>