1. Patchew
  2. QEMU
  3. [PATCH 0/6] Update Travis from Xenial to Bionic and Focal
  4. View patch

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

Thomas Huth posted 6 patches 5 years, 2 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Fam Zheng <fam@euphon.net>
[PATCH 3/6] migration: Silence compiler warning in global_state_store_running()
Posted by Thomas Huth 5 years, 2 months ago 
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
Re: [PATCH 3/6] migration: Silence compiler warning in global_state_store_running()
Posted by Cleber Rosa 5 years, 2 months ago 
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.
Re: [PATCH 3/6] migration: Silence compiler warning in global_state_store_running()
Posted by Thomas Huth 5 years, 2 months ago 
On 21/09/2020 22.39, Cleber Rosa wrote:
> 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.

 Hi Cleber,

I've hit the error here:

 https://travis-ci.com/github/huth/qemu/jobs/385871010#L2930

It seems to use the very same compiler version as you did, so that's
kind of weird... Maybe it's related to the other compiler flags, either
--enable-gprof, --enable-gcov or --disable-pie ?

 Thomas

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.