Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning
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
---
migration/global_state.c | 1 +
1 file changed, 1 insertion(+)
Reviewed-by: Eric Blake
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).
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
Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning
* Eric Blake (ebl...@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
> > ---
> > migration/global_state.c | 1 +
> > 1 file changed, 1 insertion(+)
>
> Reviewed-by: Eric Blake
>
> 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.
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 / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning
Hi
On Tue, Nov 20, 2018 at 9:22 PM Dr. David Alan Gilbert
wrote:
>
> * Eric Blake (ebl...@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
> > > ---
> > > migration/global_state.c | 1 +
> > > 1 file changed, 1 insertion(+)
> >
> > Reviewed-by: Eric Blake
> >
> > 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.
>
> 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 / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning
* Marc-André Lureau (marcandre.lur...@redhat.com) wrote:
> Hi
>
> On Tue, Nov 20, 2018 at 9:22 PM Dr. David Alan Gilbert
> wrote:
> >
> > * Eric Blake (ebl...@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
> > > > ---
> > > > migration/global_state.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > >
> > > Reviewed-by: Eric Blake
> > >
> > > 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
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 / dgilb...@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-block] [Qemu-devel] [PATCH for-3.1? 2/3] migration: fix stringop-truncation warning
On 20/11/18 16:27, 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
Reviewed-by: Philippe Mathieu-Daudé
---
migration/global_state.c | 1 +
1 file changed, 1 insertion(+)
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));
}
