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

[Eric Blake]

On 06/07/2018 01:28 PM, Ari Sundholm wrote:
> On 06/07/2018 07:13 PM, address@hidden 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