On Mon, Mar 31, 2014 at 04:01:34PM +0100, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > move size offset and number of elements math out
> > to functions, to reduce code duplication.
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > vmstate.c | 97
> > ++++++++++++++++++++++++++++++++++-----------------------------
> > 1 file changed, 52 insertions(+), 45 deletions(-)
> >
> > diff --git a/vmstate.c b/vmstate.c
> > index b689f2f..dc99e1a 100644
> > --- a/vmstate.c
> > +++ b/vmstate.c
> > @@ -10,6 +10,50 @@ static void vmstate_subsection_save(QEMUFile *f, const
> > VMStateDescription *vmsd,
> > static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription
> > *vmsd,
> > void *opaque);
> >
> > +static int vmstate_n_elems(void *opaque, VMStateField *field)
> > +{
> > + int n_elems = 1;
> > +
> > + if (field->flags & VMS_ARRAY) {
> > + n_elems = field->num;
> > + } else if (field->flags & VMS_VARRAY_INT32) {
> > + n_elems = *(int32_t *)(opaque+field->num_offset);
> > + } else if (field->flags & VMS_VARRAY_UINT32) {
> > + n_elems = *(uint32_t *)(opaque+field->num_offset);
> > + } else if (field->flags & VMS_VARRAY_UINT16) {
> > + n_elems = *(uint16_t *)(opaque+field->num_offset);
> > + } else if (field->flags & VMS_VARRAY_UINT8) {
> > + n_elems = *(uint8_t *)(opaque+field->num_offset);
> > + }
> > +
> > + return n_elems;
> > +}
> > +
> > +static int vmstate_size(void *opaque, VMStateField *field)
> > +{
> > + int size = field->size;
> > +
> > + if (field->flags & VMS_VBUFFER) {
> > + size = *(int32_t *)(opaque+field->size_offset);
> > + if (field->flags & VMS_MULTIPLY) {
> > + size *= field->size;
> > + }
> > + }
> > +
> > + return size;
> > +}
> > +
> > +static void *vmstate_base_addr(void *opaque, VMStateField *field)
> > +{
> > + void *base_addr = opaque + field->offset;
> > +
> > + if (field->flags & VMS_POINTER) {
> > + base_addr = *(void **)base_addr + field->start;
> > + }
> > +
> > + return base_addr;
> > +}
> > +
> > int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
> > void *opaque, int version_id)
> > {
> > @@ -36,30 +80,10 @@ int vmstate_load_state(QEMUFile *f, const
> > VMStateDescription *vmsd,
> > field->field_exists(opaque, version_id)) ||
> > (!field->field_exists &&
> > field->version_id <= version_id)) {
> > - void *base_addr = opaque + field->offset;
> > - int i, n_elems = 1;
> > - int size = field->size;
> > -
> > - if (field->flags & VMS_VBUFFER) {
> > - size = *(int32_t *)(opaque+field->size_offset);
> > - if (field->flags & VMS_MULTIPLY) {
> > - size *= field->size;
> > - }
> > - }
> > - if (field->flags & VMS_ARRAY) {
> > - n_elems = field->num;
> > - } else if (field->flags & VMS_VARRAY_INT32) {
> > - n_elems = *(int32_t *)(opaque+field->num_offset);
> > - } else if (field->flags & VMS_VARRAY_UINT32) {
> > - n_elems = *(uint32_t *)(opaque+field->num_offset);
> > - } else if (field->flags & VMS_VARRAY_UINT16) {
> > - n_elems = *(uint16_t *)(opaque+field->num_offset);
> > - } else if (field->flags & VMS_VARRAY_UINT8) {
> > - n_elems = *(uint8_t *)(opaque+field->num_offset);
> > - }
> > - if (field->flags & VMS_POINTER) {
> > - base_addr = *(void **)base_addr + field->start;
> > - }
> > + void *base_addr = vmstate_base_addr(opaque, field);
> > + int i, n_elems = vmstate_n_elems(opaque, field);
> > + int size = vmstate_size(opaque, field);
> > +
> > for (i = 0; i < n_elems; i++) {
> > void *addr = base_addr + size * i;
> >
> > @@ -102,27 +126,10 @@ void vmstate_save_state(QEMUFile *f, const
> > VMStateDescription *vmsd,
> > while (field->name) {
> > if (!field->field_exists ||
> > field->field_exists(opaque, vmsd->version_id)) {
> > - void *base_addr = opaque + field->offset;
> > - int i, n_elems = 1;
> > - int size = field->size;
> > -
> > - if (field->flags & VMS_VBUFFER) {
> > - size = *(int32_t *)(opaque+field->size_offset);
> > - if (field->flags & VMS_MULTIPLY) {
> > - size *= field->size;
> > - }
> > - }
> > - if (field->flags & VMS_ARRAY) {
> > - n_elems = field->num;
> > - } else if (field->flags & VMS_VARRAY_INT32) {
> > - n_elems = *(int32_t *)(opaque+field->num_offset);
> > - } else if (field->flags & VMS_VARRAY_UINT32) {
> > - n_elems = *(uint32_t *)(opaque+field->num_offset);
> > - } else if (field->flags & VMS_VARRAY_UINT16) {
> > - n_elems = *(uint16_t *)(opaque+field->num_offset);
> > - } else if (field->flags & VMS_VARRAY_UINT8) {
> > - n_elems = *(uint8_t *)(opaque+field->num_offset);
> > - }
> > + void *base_addr = vmstate_base_addr(opaque, field);
> > + int i, n_elems = vmstate_n_elems(opaque, field);
> > + int size = vmstate_size(opaque, field);
> > +
> > if (field->flags & VMS_POINTER) {
> > base_addr = *(void **)base_addr + field->start;
> > }
>
> Hmm, shouldn't those last 3 lines be deleted as well - the logic is
> now in vmstate_base_addr?
>
>
> Dave
So it should, thanks.
> > --
> > MST
> >
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
