Re: [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Hi
> 
> On Tue, Nov 20, 2018 at 9:22 PM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Eric Blake (eblake@redhat.com) wrote:
> > > On 11/20/18 9:27 AM, Marc-André Lureau wrote:
> > > > Adding an assert is enough to silence GCC.
> > > >
> > > > ~/src/qemu/migration/global_state.c: In function 'global_state_store_running':
> > > > ~/src/qemu/migration/global_state.c:45:5: error: 'strncpy' specified bound 100 equals destination size [-Werror=stringop-truncation]
> > > >       strncpy((char *)global_state.runstate,
> > > >       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >              state, sizeof(global_state.runstate));
> > > >              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > cc1: all warnings being treated as errors
> > > >
> > > > (alternatively, we could hard-code "running")
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >   migration/global_state.c | 1 +
> > > >   1 file changed, 1 insertion(+)
> > >
> > > Reviewed-by: Eric Blake <eblake@redhat.com>
> > >
> > > I think this is safe for 3.1, but I know the migration code is particularly
> > > wary of assert()s, even when they are non-triggerable (a 100-byte buffer at
> > > global_state.runstate is big enough for ALL of the run states, not just
> > > RUN_STATE_RUNNING).
> >
> > That's OK; the universe would have to be particularly broken to trigger
> > that one, and it's in no way connected with any state, so it would
> > trigger on even the most basic tests.
> >
> > However, I wonder if this fixes the problem on mingw builds - windows
> > asserts are not marked as noreturn.
> 
> On f29, with mingw64-gcc-8.2.0-3.fc29.x86_64, it fixes the warning.

OK, fine.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

> >
> > Dave
> >
> > > >
> > > > diff --git a/migration/global_state.c b/migration/global_state.c
> > > > index 8e8ab5c51e..01805c567a 100644
> > > > --- a/migration/global_state.c
> > > > +++ b/migration/global_state.c
> > > > @@ -42,6 +42,7 @@ int global_state_store(void)
> > > >   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));
> > > >   }
> > > >
> > >
> > > --
> > > Eric Blake, Principal Software Engineer
> > > Red Hat, Inc.           +1-919-301-3266
> > > Virtualization:  qemu.org | libvirt.org
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK