On Tue, Sep 20, 2022 at 7:25 PM Markus Armbruster <arm...@redhat.com> wrote:
>
> Philippe Mathieu-Daudé <f4...@amsat.org> writes:
>
> > GCC 8 added a -Wstringop-truncation warning:
> >
> > The -Wstringop-truncation warning added in GCC 8.0 via r254630 for
> > bug 81117 is specifically intended to highlight likely unintended
> > uses of the strncpy function that truncate the terminating NUL
> > character from the source string.
> >
> > Here the next line indeed unconditionally zeroes the last byte, so
> > we can call strncpy() on the buffer size less the last byte.
>
> Actually, the buffer is all zero to begin with, so we could do this even
> without the next line's assignment.
>
Yes, I think we can remove the next line's assignment.
> > This
> > fixes when using gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0:
> >
> > [42/666] Compiling C object subprojects/libvduse/libvduse.a.p/libvduse.c.o
> > FAILED: subprojects/libvduse/libvduse.a.p/libvduse.c.o
> > cc -m64 -mcx16 -Isubprojects/libvduse/libvduse.a.p -Isubprojects/libvduse
> > -I../../subprojects/libvduse [...] -o
> > subprojects/libvduse/libvduse.a.p/libvduse.c.o -c
> > ../../subprojects/libvduse/libvduse.c
> > In file included from /usr/include/string.h:495,
> > from ../../subprojects/libvduse/libvduse.c:24:
> > In function ‘strncpy’,
> > inlined from ‘vduse_dev_create’ at
> > ../../subprojects/libvduse/libvduse.c:1312:5:
> > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
> > ‘__builtin_strncpy’ specified bound 256 equals destination size
> > [-Werror=stringop-truncation]
> > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos
> > (__dest));
> > |
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> > ninja: build stopped: cannot make progress due to previous errors.
> >
> > Fixes: d9cf16c0be ("libvduse: Replace strcpy() with strncpy()")
> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>
> The subject feels a bit too alarming to me. This patch suppresses a
> warning, no less, no more. Behavior doesn't change. Perhaps
>
> libvduse: Avoid warning about dangerous use of strncpy()
>
> > ---
> > Cc: Xie Yongji <xieyon...@bytedance.com>
> > Cc: Markus Armbruster <arm...@redhat.com>
> > Cc: Kevin Wolf <kw...@redhat.com>
> >
> > RFC: Any better idea? We can't use strpadcpy() because libvduse
> > doesn't depend on QEMU.
>
> There's no need for padding: the destination calloc'ed. So, pstrcpy()
> would do, but it's just as unavailable. Can we use GLib? There's
> g_strlcpy().
>
> Outside this patch's scope: is silent truncation what we want?
>
Actually silent truncation would not happen since we called
vduse_name_is_invalid() before.
static inline bool vduse_name_is_invalid(const char *name)
{
return strlen(name) >= VDUSE_NAME_MAX || strstr(name, "..");
}
Thanks,
Yongji
