Re: [PATCH 3/6] migration: Silence compiler warning in global_state_store_running()

[Cleber Rosa <crosa@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4341 bytes --]

On Fri, Sep 18, 2020 at 12:34:27PM +0200, Thomas Huth wrote:
> GCC 9.3.0 on Ubuntu complains:
> 
> In file included from /usr/include/string.h:495,
>                  from /home/travis/build/huth/qemu/include/qemu/osdep.h:87,
>                  from ../migration/global_state.c:13:
> In function ‘strncpy’,
>     inlined from ‘global_state_store_running’ at ../migration/global_state.c:47:5:
> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
>  ‘__builtin_strncpy’ specified bound 100 equals destination size [-Werror=stringop-truncation]
>   106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> ... but we apparently really want to do a strncpy here - the size is already
> checked with the assert() statement right in front of it. To silence the
> warning, simply replace it with our strpadcpy() function.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com> (two years ago)
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  migration/global_state.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 25311479a4..a33947ca32 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -44,8 +44,8 @@ void global_state_store_running(void)
>  {
>      const char *state = RunState_str(RUN_STATE_RUNNING);
>      assert(strlen(state) < sizeof(global_state.runstate));
> -    strncpy((char *)global_state.runstate,
> -           state, sizeof(global_state.runstate));
> +    strpadcpy((char *)global_state.runstate, sizeof(global_state.runstate),
> +              state, '\0');
>  }
>  
>  bool global_state_received(void)
> -- 
> 2.18.2
> 
> 

Hi Thomas,

FIY, I couldn't reproduce the complaint from GCC.  I've tested it on focal,
"gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0", with QEMU 5df6c87e8.

After a succesfull "configure --target-list=x86_64-softmmu && meson
compile" build, I tried to manually enable meson's "werror" option,
and found no difference.

Then, I manually ran gcc, with a couple of "-Werror" variations, such
as:

cc -Ilibcommon.fa.p -I. -I../../src/qemu -Iqapi -Itrace -Iui \
   -Iui/shader -I/usr/include/libpng16 -I/usr/include/libmount \
   -I/usr/include/blkid -I/usr/include/glib-2.0 \
   -I/usr/lib/x86_64-linux-gnu/glib-2.0/include \
   -I/usr/include/gio-unix-2.0 -I/root/src/qemu/slirp/src -Islirp/src \
   -I/usr/include/pixman-1 -I/usr/include/gtk-3.0 \
   -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 \
   -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include \
   -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/fribidi \
   -I/usr/include/harfbuzz -I/usr/include/atk-1.0 -I/usr/include/uuid \
   -I/usr/include/freetype2 -I/usr/include/gdk-pixbuf-2.0 -Ilinux-headers \
   -fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror \
   -Werror=stringop-truncation -std=gnu99 -O2 -g -U_FORTIFY_SOURCE \
   -D_FORTIFY_SOURCE=2 -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 \
   -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef \
   -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common \
   -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits \
   -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers \
   -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined \
   -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi \
   -fstack-protector-strong -iquote /root/src/qemu/tcg/i386 -isystem \
   /root/src/qemu/linux-headers -iquote . -iquote /root/src/qemu -iquote \
   /root/src/qemu/accel/tcg -iquote /root/src/qemu/include -iquote \
   /root/src/qemu/disas/libvixl -pthread -fPIC -MD -MQ \
   libcommon.fa.p/migration_global_state.c.o -MF \
   libcommon.fa.p/migration_global_state.c.o.d -o \
   libcommon.fa.p/migration_global_state.c.o -c \
   ../../src/qemu/migration/global_state.c

But I could not trigger the warning (and thus error).  The change here
looks good, but I thought I should let you know, and maybe I'm missing
something obvious.

Thanks,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]