All of lore.kernel.org
 help / color / mirror / Atom feed
From: Halil Pasic <pasic@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: Amit Shah <amit.shah@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	Guenther Hutzl <hutzl@linux.vnet.ibm.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Halil Pasic <pasic@linux.vnet.ibm.com>
Subject: [Qemu-devel] [RFC PATCH v2 5/8] migration/vmstate: renames in (load|save)_state
Date: Tue,  8 Nov 2016 10:56:00 +0100	[thread overview]
Message-ID: <20161108095603.72301-6-pasic@linux.vnet.ibm.com> (raw)
In-Reply-To: <20161108095603.72301-1-pasic@linux.vnet.ibm.com>

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(-)

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

  parent reply	other threads:[~2016-11-08  9:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-08  9:55 [Qemu-devel] [RFC PATCH v2 0/8] VMS_ARRAY_OF_POINTER with null pointers Halil Pasic
2016-11-08  9:55 ` [Qemu-devel] [RFC PATCH v2 1/8] tests/test-vmstate.c: add vBuffer test Halil Pasic
2016-11-08  9:55 ` [Qemu-devel] [RFC PATCH v2 2/8] migration: drop unused VMStateField.start Halil Pasic
2016-11-08  9:55 ` [Qemu-devel] [RFC PATCH v2 3/8] tests/test-vmstate.c: add save_buffer util func Halil Pasic
2016-11-08  9:55 ` [Qemu-devel] [RFC PATCH v2 4/8] tests/test-vmstate.c: add array of pointer to struct Halil Pasic
2016-11-08  9:56 ` Halil Pasic [this message]
2016-12-15 11:55   ` [Qemu-devel] [RFC PATCH v2 5/8] migration/vmstate: renames in (load|save)_state Dr. David Alan Gilbert
2016-11-08  9:56 ` [Qemu-devel] [RFC PATCH v2 6/8] migration/vmstate: split up vmstate_base_addr Halil Pasic
2016-12-15 13:29   ` Dr. David Alan Gilbert
2016-12-16 15:57     ` Halil Pasic
2016-12-16 19:47       ` Dr. David Alan Gilbert
2016-11-08  9:56 ` [Qemu-devel] [RFC PATCH v2 7/8] migration/vmstate: fix array of pointers to struct Halil Pasic
2016-12-15 12:33   ` Dr. David Alan Gilbert
2017-01-31 15:17     ` Halil Pasic
2017-02-01 18:18       ` Dr. David Alan Gilbert
2016-11-08  9:56 ` [Qemu-devel] [RFC PATCH v2 8/8] tests/test-vmstate.c: add array of pointers to struct with NULL Halil Pasic
2016-12-15 12:52   ` Dr. David Alan Gilbert
2016-12-15 13:31 ` [Qemu-devel] [RFC PATCH v2 0/8] VMS_ARRAY_OF_POINTER with null pointers Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161108095603.72301-6-pasic@linux.vnet.ibm.com \
    --to=pasic@linux.vnet.ibm.com \
    --cc=amit.shah@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=hutzl@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.