Re: [Qemu-devel] [PATCH v3 00/10] New block driver: blklogwrites

[Eric Blake <eblake@redhat.com>]

On 06/07/2018 01:28 PM, Ari Sundholm wrote:
> On 06/07/2018 07:13 PM, no-reply@patchew.org wrote:
>> Hi,
>>
>> This series failed build test on s390x host. Please find the details 
>> below.
>>

>> /var/tmp/patchew-tester-tmp-8bz4jnox/src/block/blklogwrites.c: In 
>> function ‘blk_log_writes_refresh_filename’:
>> /var/tmp/patchew-tester-tmp-8bz4jnox/src/block/blklogwrites.c:136:32: 
>> error: ‘%s’ directive output may be truncated writing up to 4095 bytes 
>> into a region of size 4083 [-Werror=format-truncation=]
>>                    "blklogwrites:%s:%s",
>>                                  ^~
>> In file included from /usr/include/stdio.h:939:0,
>>                   from 
>> /var/tmp/patchew-tester-tmp-8bz4jnox/src/include/qemu/osdep.h:68,
>>                   from 
>> /var/tmp/patchew-tester-tmp-8bz4jnox/src/block/blklogwrites.c:12:
>> /usr/include/bits/stdio2.h:64:10: note: ‘__builtin___snprintf_chk’ 
>> output between 15 and 8205 bytes into a destination of size 4096
>>     return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
>>            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>          __bos (__s), __fmt, __va_arg_pack ());
>>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make: *** [/var/tmp/patchew-tester-tmp-8bz4jnox/src/rules.mak:69: 
>> block/blklogwrites.o] Error 1
>> make: *** Waiting for unfinished jobs....
>>    CC      block/qapi.o
>> === OUTPUT END ===
>>
>> Test command exited with code: 2
>>
> 
> Given that blkverify.c has a similar snprintf() call, with the exception 
> that it checks the return value in case the string was truncated, am I 
> safe in assuming that adding a check for the return value of snprintf() 
> fixes this one? I don't really see anything else I could do about the 
> error.

Well, ideally we'd g_strdup_printf() the string (and g_free() it later) 
rather than use a fixed-size array, if we HAVE to produce a 
legacy-format name in the first place.  But if we stick with a 
fixed-width buffer, perhaps checking for snprintf() truncation, and 
returning NULL in that case, is enough to force a fallback to a 
pseudo-JSON string so that the end user doesn't lose information and the 
compiler doesn't complain about failure to check for truncation.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org