[RFC,v2,5/8] migration/vmstate: renames in (load|save)_state

The vmstate_(load|save)_state start out with an a void *opaque pointing
to some struct, and manipulate one or more elements of one field within
that struct.

First the field within the struct is pinpointed as opaque + offset, then
if this is a pointer the pointer is dereferenced to obtain a pointer to
the first element of the vmstate field. Pointers to further elements if
any are calculated as first_element + i * element_size (where i is the
zero based index of the element in question).

Currently base_addr and addr is used as a variable name for the pointer
to the first element and the pointer to the current element being
processed. This is suboptimal because base_addr is somewhat
counter-intuitive (because obtained as base + offset) and both base_addr
and addr not very descriptive (that we have a pointer should be clear
from the fact that it is declared as a pointer).

Let make things easier to understand by renaming base_addr to first_elem
and addr to curr_elem. This has the additional benefit of harmonizing
with other names within the scope (n_elems, vmstate_n_elems).

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>

--

This patch roots in the difficulties I faced when trying to figure out
the code in question. In the meanwhile I'm quite fine with the current
names because I have it already figured out (after a couple of months
not looking on this code however, who knows). Thus my main motivation
is making things easier for the next new guy, but if the old guys say
not worth it I can very well accept that too.
---
 migration/vmstate.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

* Halil Pasic (pasic@linux.vnet.ibm.com) wrote:
> The vmstate_(load|save)_state start out with an a void *opaque pointing
> to some struct, and manipulate one or more elements of one field within
> that struct.
> 
> First the field within the struct is pinpointed as opaque + offset, then
> if this is a pointer the pointer is dereferenced to obtain a pointer to
> the first element of the vmstate field. Pointers to further elements if
> any are calculated as first_element + i * element_size (where i is the
> zero based index of the element in question).
> 
> Currently base_addr and addr is used as a variable name for the pointer
> to the first element and the pointer to the current element being
> processed. This is suboptimal because base_addr is somewhat
> counter-intuitive (because obtained as base + offset) and both base_addr
> and addr not very descriptive (that we have a pointer should be clear
> from the fact that it is declared as a pointer).
> 
> Let make things easier to understand by renaming base_addr to first_elem
> and addr to curr_elem. This has the additional benefit of harmonizing
> with other names within the scope (n_elems, vmstate_n_elems).
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> --
> 
> This patch roots in the difficulties I faced when trying to figure out
> the code in question. In the meanwhile I'm quite fine with the current
> names because I have it already figured out (after a couple of months
> not looking on this code however, who knows). Thus my main motivation
> is making things easier for the next new guy, but if the old guys say
> not worth it I can very well accept that too.
> ---
>  migration/vmstate.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 8767e40..a86fb7e 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -108,21 +108,21 @@ 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 = vmstate_base_addr(opaque, field, true);
> +            void *first_elem = vmstate_base_addr(opaque, field, true);
>              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;
> +                void *curr_elem = first_elem + size * i;
>  
>                  if (field->flags & VMS_ARRAY_OF_POINTER) {
> -                    addr = *(void **)addr;
> +                    curr_elem = *(void **)curr_elem;
>                  }
>                  if (field->flags & VMS_STRUCT) {
> -                    ret = vmstate_load_state(f, field->vmsd, addr,
> +                    ret = vmstate_load_state(f, field->vmsd, curr_elem,
>                                               field->vmsd->version_id);
>                  } else {
> -                    ret = field->info->get(f, addr, size);
> +                    ret = field->info->get(f, curr_elem, size);
>  
>                  }
>                  if (ret >= 0) {
> @@ -310,25 +310,25 @@ 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 = vmstate_base_addr(opaque, field, false);
> +            void *first_elem = vmstate_base_addr(opaque, field, false);
>              int i, n_elems = vmstate_n_elems(opaque, field);
>              int size = vmstate_size(opaque, field);
>              int64_t old_offset, written_bytes;
>              QJSON *vmdesc_loop = vmdesc;
>  
>              for (i = 0; i < n_elems; i++) {
> -                void *addr = base_addr + size * i;
> +                void *curr_elem = first_elem + size * i;
>  
>                  vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
>                  old_offset = qemu_ftell_fast(f);
> -
>                  if (field->flags & VMS_ARRAY_OF_POINTER) {
> -                    addr = *(void **)addr;
> +                    assert(curr_elem);
> +                    curr_elem = *(void **)curr_elem;
>                  }
>                  if (field->flags & VMS_STRUCT) {
> -                    vmstate_save_state(f, field->vmsd, addr, vmdesc_loop);
> +                    vmstate_save_state(f, field->vmsd, curr_elem, vmdesc_loop);
>                  } else {
> -                    field->info->put(f, addr, size);
> +                    field->info->put(f, curr_elem, size);
>                  }
>  
>                  written_bytes = qemu_ftell_fast(f) - old_offset;
> -- 
> 2.8.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK