diff mbox

[RFC,v2,1/5] vmstate: reduce code duplication

Message ID 1395671853-2685-2-git-send-email-mst@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin March 24, 2014, 2:37 p.m. UTC
move size offset and number of elements math out
to functions, to reduce code duplication.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 vmstate.c | 97 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 52 insertions(+), 45 deletions(-)

Comments

Dr. David Alan Gilbert March 24, 2014, 3:56 p.m. UTC | #1
* 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 <dgilbert@redhat.com>

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 <mst@redhat.com>
> ---
>  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
diff mbox

Patch

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;
             }