| Message ID | 20200916171913.148681-1-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: Silence compiler warning in global_state_store_running() | expand |
On 9/16/20 12:19 PM, 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 the strncpy here. Silence the > warning with QEMU_NONSTRING. > > 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..f1355d7d97 100644 > --- a/migration/global_state.c > +++ b/migration/global_state.c > @@ -43,9 +43,9 @@ int global_state_store(void) > void global_state_store_running(void) > { > const char *state = RunState_str(RUN_STATE_RUNNING); > + QEMU_NONSTRING char *dest = (char *)global_state.runstate; Do we still need the cast to char*? > assert(strlen(state) < sizeof(global_state.runstate)); > - strncpy((char *)global_state.runstate, > - state, sizeof(global_state.runstate)); > + strncpy(dest, state, sizeof(global_state.runstate)); > } > Otherwise, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On 16/09/2020 19.19, 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 the strncpy here. Silence the > warning with QEMU_NONSTRING. > > 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..f1355d7d97 100644 > --- a/migration/global_state.c > +++ b/migration/global_state.c > @@ -43,9 +43,9 @@ int global_state_store(void) > void global_state_store_running(void) > { > const char *state = RunState_str(RUN_STATE_RUNNING); > + QEMU_NONSTRING char *dest = (char *)global_state.runstate; > assert(strlen(state) < sizeof(global_state.runstate)); > - strncpy((char *)global_state.runstate, > - state, sizeof(global_state.runstate)); > + strncpy(dest, state, sizeof(global_state.runstate)); > } Darn, I was sending too fast here, sorry, but seems like this does *not* fix the issue with GCC 9.3: https://travis-ci.com/github/huth/qemu/jobs/385871010#L2930 ... so maybe we should simply switch to strpadcpy() instead? Thomas
On 17/09/20 08:30, Thomas Huth wrote: > On 16/09/2020 19.19, 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 the strncpy here. Silence the >> warning with QEMU_NONSTRING. >> >> 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..f1355d7d97 100644 >> --- a/migration/global_state.c >> +++ b/migration/global_state.c >> @@ -43,9 +43,9 @@ int global_state_store(void) >> void global_state_store_running(void) >> { >> const char *state = RunState_str(RUN_STATE_RUNNING); >> + QEMU_NONSTRING char *dest = (char *)global_state.runstate; >> assert(strlen(state) < sizeof(global_state.runstate)); >> - strncpy((char *)global_state.runstate, >> - state, sizeof(global_state.runstate)); >> + strncpy(dest, state, sizeof(global_state.runstate)); >> } > > Darn, I was sending too fast here, sorry, but seems like this does *not* > fix the issue with GCC 9.3: > > https://travis-ci.com/github/huth/qemu/jobs/385871010#L2930 > > ... so maybe we should simply switch to strpadcpy() instead? Yes, and probably do so everywhere that strncpy is used. Paolo
On 17/09/2020 09.18, Paolo Bonzini wrote: > On 17/09/20 08:30, Thomas Huth wrote: >> On 16/09/2020 19.19, 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 the strncpy here. Silence the >>> warning with QEMU_NONSTRING. >>> >>> 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..f1355d7d97 100644 >>> --- a/migration/global_state.c >>> +++ b/migration/global_state.c >>> @@ -43,9 +43,9 @@ int global_state_store(void) >>> void global_state_store_running(void) >>> { >>> const char *state = RunState_str(RUN_STATE_RUNNING); >>> + QEMU_NONSTRING char *dest = (char *)global_state.runstate; >>> assert(strlen(state) < sizeof(global_state.runstate)); >>> - strncpy((char *)global_state.runstate, >>> - state, sizeof(global_state.runstate)); >>> + strncpy(dest, state, sizeof(global_state.runstate)); >>> } >> >> Darn, I was sending too fast here, sorry, but seems like this does *not* >> fix the issue with GCC 9.3: >> >> https://travis-ci.com/github/huth/qemu/jobs/385871010#L2930 >> >> ... so maybe we should simply switch to strpadcpy() instead? > > Yes, and probably do so everywhere that strncpy is used. I think the trick with QEMU_NONSTRING should work fine in all cases where we have a real array, see e.g. buf[] in find_vdi_name() in block/sheepdog.c. It just does not seem to work in case you have a char* pointer instead of an array... Thomas
diff --git a/migration/global_state.c b/migration/global_state.c index 25311479a4..f1355d7d97 100644 --- a/migration/global_state.c +++ b/migration/global_state.c @@ -43,9 +43,9 @@ int global_state_store(void) void global_state_store_running(void) { const char *state = RunState_str(RUN_STATE_RUNNING); + QEMU_NONSTRING char *dest = (char *)global_state.runstate; assert(strlen(state) < sizeof(global_state.runstate)); - strncpy((char *)global_state.runstate, - state, sizeof(global_state.runstate)); + strncpy(dest, state, sizeof(global_state.runstate)); } bool global_state_received(void)
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 the strncpy here. Silence the warning with QEMU_NONSTRING. Signed-off-by: Thomas Huth <thuth@redhat.com> --- migration/global_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)