[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 01/30] vmstate: reduce code duplication
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v4 01/30] vmstate: reduce code duplication |
Date: |
Mon, 31 Mar 2014 16:01:34 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
* Michael S. Tsirkin (address@hidden) wrote:
> move size offset and number of elements math out
> to functions, to reduce code duplication.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> 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
> --
> MST
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v4 00/30] qemu state loading issues, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 01/30] vmstate: reduce code duplication, Michael S. Tsirkin, 2014/03/31
- Re: [Qemu-devel] [PATCH v4 01/30] vmstate: reduce code duplication,
Dr. David Alan Gilbert <=
- [Qemu-devel] [PATCH v4 02/30] vmstate: add VMS_MUST_EXIST, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 03/30] vmstate: add VMSTATE_VALIDATE, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 04/30] virtio-net: fix buffer overflow on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 05/30] virtio-net: out-of-bounds buffer write on load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 06/30] virtio-net: out-of-bounds buffer write on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 07/30] virtio: out-of-bounds buffer write on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 09/30] hpet: fix buffer overrun on invalid state load, Michael S. Tsirkin, 2014/03/31