From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34870) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WS7F7-0005oC-39 for qemu-devel@nongnu.org; Mon, 24 Mar 2014 11:56:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WS7Ez-0000FO-CR for qemu-devel@nongnu.org; Mon, 24 Mar 2014 11:56:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47109) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WS7Ez-0000FB-3v for qemu-devel@nongnu.org; Mon, 24 Mar 2014 11:56:45 -0400 Date: Mon, 24 Mar 2014 15:56:38 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20140324155637.GF3829@work-vm> References: <1395671853-2685-1-git-send-email-mst@redhat.com> <1395671853-2685-2-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1395671853-2685-2-git-send-email-mst@redhat.com> Subject: Re: [Qemu-devel] [RFC v2 1/5] vmstate: reduce code duplication List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Peter Maydell , qemu-devel@nongnu.org * Michael S. Tsirkin (mst@redhat.com) wrote: > move size offset and number of elements math out > to functions, to reduce code duplication. Reviewed-by: Dr. David Alan Gilbert If this was new code I would have rejected the use of signed 'int' for something counting the number of elements and size; but this is just a rearrange, so lets fight that another time. > > Signed-off-by: Michael S. Tsirkin > --- > vmstate.c | 97 ++++++++++++++++++++++++++++++++++----------------------------- > 1 file changed, 52 insertions(+), 45 deletions(-) > > diff --git a/vmstate.c b/vmstate.c > index c61b0e5..18b3732 100644 > --- a/vmstate.c > +++ b/vmstate.c > @@ -9,6 +9,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) > { > @@ -38,30 +82,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; > > @@ -103,27 +127,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; > } > -- > MST > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK