diff mbox series

clock-vmstate: Add missing END_OF_LIST

Message ID 20220111101934.115028-1-dgilbert@redhat.com (mailing list archive)
State New, archived
Headers show
Series clock-vmstate: Add missing END_OF_LIST | expand

Commit Message

Dr. David Alan Gilbert Jan. 11, 2022, 10:19 a.m. UTC 
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv

Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/core/clock-vmstate.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Peter Maydell Jan. 11, 2022, 10:30 a.m. UTC | #1 
On Tue, 11 Jan 2022 at 10:19, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv
>
> Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/core/clock-vmstate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
> index 9d9174ffbd..7eccb6d4ea 100644
> --- a/hw/core/clock-vmstate.c
> +++ b/hw/core/clock-vmstate.c
> @@ -44,6 +44,7 @@ const VMStateDescription vmstate_muldiv = {
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(multiplier, Clock),
>          VMSTATE_UINT32(divider, Clock),
> +        VMSTATE_END_OF_LIST()
>      },
>  };

Oops :-(

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

We could maybe make this kind of bug more likely to be detected by
instead of having the terminator be an "all zeros" VMStateField,
having it check both for name == NULL and for a magic number in the
VMStateFlags field. That way (assuming something in "make check"
causes us to do a scan through every registered vmstate struct)
forgetting the terminator will be likely to cause us to crash or hang
rather than finding some 0 data and thinking that's the
terminator.

thanks
-- PMM
Peter Maydell Jan. 11, 2022, 12:11 p.m. UTC | #2 
On Tue, 11 Jan 2022 at 10:19, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv
>
> Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/core/clock-vmstate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
> index 9d9174ffbd..7eccb6d4ea 100644
> --- a/hw/core/clock-vmstate.c
> +++ b/hw/core/clock-vmstate.c
> @@ -44,6 +44,7 @@ const VMStateDescription vmstate_muldiv = {
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(multiplier, Clock),
>          VMSTATE_UINT32(divider, Clock),
> +        VMSTATE_END_OF_LIST()
>      },
>  };

Cc: qemu-stable@nongnu.org
seems appropriate here ?

thanks
-- PMM
Philippe Mathieu-Daudé Jan. 11, 2022, 1:01 p.m. UTC | #3 
On 1/11/22 11:19, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv
> 
> Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")

Doh I missed this too :(

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/core/clock-vmstate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
> index 9d9174ffbd..7eccb6d4ea 100644
> --- a/hw/core/clock-vmstate.c
> +++ b/hw/core/clock-vmstate.c
> @@ -44,6 +44,7 @@ const VMStateDescription vmstate_muldiv = {
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(multiplier, Clock),
>          VMSTATE_UINT32(divider, Clock),
> +        VMSTATE_END_OF_LIST()
>      },
>  };
>
Alex Bennée Jan. 11, 2022, 1:40 p.m. UTC | #4 
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv
>
> Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Dr. David Alan Gilbert Jan. 11, 2022, 2:01 p.m. UTC | #5 
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Tue, 11 Jan 2022 at 10:19, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> >
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv
> >
> > Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hw/core/clock-vmstate.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
> > index 9d9174ffbd..7eccb6d4ea 100644
> > --- a/hw/core/clock-vmstate.c
> > +++ b/hw/core/clock-vmstate.c
> > @@ -44,6 +44,7 @@ const VMStateDescription vmstate_muldiv = {
> >      .fields = (VMStateField[]) {
> >          VMSTATE_UINT32(multiplier, Clock),
> >          VMSTATE_UINT32(divider, Clock),
> > +        VMSTATE_END_OF_LIST()
> >      },
> >  };
> 
> Oops :-(
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> We could maybe make this kind of bug more likely to be detected by
> instead of having the terminator be an "all zeros" VMStateField,
> having it check both for name == NULL and for a magic number in the
> VMStateFlags field. That way (assuming something in "make check"
> causes us to do a scan through every registered vmstate struct)
> forgetting the terminator will be likely to cause us to crash or hang
> rather than finding some 0 data and thinking that's the
> terminator.

Yeh, let me see what I can come up with for that.

Dave

> thanks
> -- PMM
>
Luc Michel Jan. 12, 2022, 7:48 a.m. UTC | #6 
On 10:19 Tue 11 Jan     , Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv
> 
> Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
>  hw/core/clock-vmstate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
> index 9d9174ffbd..7eccb6d4ea 100644
> --- a/hw/core/clock-vmstate.c
> +++ b/hw/core/clock-vmstate.c
> @@ -44,6 +44,7 @@ const VMStateDescription vmstate_muldiv = {
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(multiplier, Clock),
>          VMSTATE_UINT32(divider, Clock),
> +        VMSTATE_END_OF_LIST()
>      },
>  };
>  
> -- 
> 2.34.1
> 

--
Dr. David Alan Gilbert March 2, 2022, 10:49 a.m. UTC | #7 
* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add the missing VMSTATE_END_OF_LIST to vmstate_muldiv

Queued

> 
> Fixes: 99abcbc7600 ("clock: Provide builtin multiplier/divider")
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/core/clock-vmstate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
> index 9d9174ffbd..7eccb6d4ea 100644
> --- a/hw/core/clock-vmstate.c
> +++ b/hw/core/clock-vmstate.c
> @@ -44,6 +44,7 @@ const VMStateDescription vmstate_muldiv = {
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(multiplier, Clock),
>          VMSTATE_UINT32(divider, Clock),
> +        VMSTATE_END_OF_LIST()
>      },
>  };
>  
> -- 
> 2.34.1
> 
>
diff mbox series

Patch

diff --git a/hw/core/clock-vmstate.c b/hw/core/clock-vmstate.c
index 9d9174ffbd..7eccb6d4ea 100644
--- a/hw/core/clock-vmstate.c
+++ b/hw/core/clock-vmstate.c
@@ -44,6 +44,7 @@  const VMStateDescription vmstate_muldiv = {
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(multiplier, Clock),
         VMSTATE_UINT32(divider, Clock),
+        VMSTATE_END_OF_LIST()
     },
 };