* Halil Pasic (pa...@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 <pa...@linux.vnet.ibm.com>
Reviewed-by: Dr. David Alan Gilbert <dgilb...@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 / dgilb...@redhat.com / Manchester, UK
