* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Thu, 25 Jul 2019 at 18:27, Dr. David Alan Gilbert > <dgilb...@redhat.com> wrote: > > > > * Peter Maydell (peter.mayd...@linaro.org) wrote: > > > The VMSTATE_STRUCT_VARRAY_UINT32 macro is intended to handle > > > migrating a field which is an array of structs, but where instead of > > > migrating the entire array we only migrate a variable number of > > > elements of it. > > > > > > The VMSTATE_STRUCT_VARRAY_POINTER_UINT32 macro is intended to handle > > > migrating a field which is of pointer type, and points to a > > > dynamically allocated array of structs of variable size. > > > > > > We weren't actually checking that the field passed to > > > VMSTATE_STRUCT_VARRAY_UINT32 really is an array, with the result that > > > accidentally using it where the _POINTER_ macro was intended would > > > compile but silently corrupt memory on migration. > > > > > > Add type-checking that enforces that the field passed in is > > > really of the right array type. This applies to all the VMSTATE > > > macros which use flags including VMS_VARRAY_* but not VMS_POINTER. > > > > > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > > > > --- > > > include/migration/vmstate.h | 27 +++++++++++++++++++++------ > > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > > index ca68584eba4..2df333c3612 100644 > > > --- a/include/migration/vmstate.h > > > +++ b/include/migration/vmstate.h > > > @@ -227,8 +227,19 @@ extern const VMStateInfo vmstate_info_bitmap; > > > extern const VMStateInfo vmstate_info_qtailq; > > > > > > #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > > > +/* Check that t2 is an array of t1 of size n */ > > > #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0) > > > > I'd have to admit I don't understand why that does what you say; > > I'd expected something to index a t2 pointer with [n]. > > Note that this is just a comment describing what the existing > macro does, as a way to distinguish its job from that of the > new macro I'm adding. > > What happens here is that t2 is a type like "foo [32]", ie > it is an array type already. t1 is the base 'foo' type; so the macro > is checking that t1[n] matches t2, where n is passed in to us > and must match the declared array size of the field (32 in > my example). (In C the size of the array is carried around as > part of its type, and must match on both sides of the expression; > so if you pass in the name of an array field that's the wrong size the > type check will fail, which is what we want.)
Ah, OK that makes sense; what it really needs is that example to make me realise that t2 was already the array. Dave > > However, for the rest of it, from migration I'm happy: > > > > Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > > > > given it's just fixing an ARM bug, and given it'll blow up straight away > > I think it's OK for 4.1; the only risk is if we find a compiler we don't > > like. > > thanks > -- PMM -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
