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".
Paolo
> Regardless,
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
>
