[Qemu-devel] [PULL v2 2/4] qom: Change object property iterator API contract

["Andreas Färber" <afaerber@suse.de>]

From: "Daniel P. Berrange" <berrange@redhat.com>

Currently the ObjectProperty iterator API works as follows:

  ObjectPropertyIterator *iter;

  iter = object_property_iter_init(obj);
  while ((prop = object_property_iter_next(iter))) {
     ...
  }
  object_property_iter_free(iter);

This has the benefit that the ObjectPropertyIterator struct
can be opaque, but has the downside that callers need to
explicitly call a free function. It is also not in keeping
with iterator style used elsewhere in QEMU/GLib2.

This patch changes the API to use stack allocation instead:

  ObjectPropertyIterator iter;

  object_property_iter_init(&iter, obj);
  while ((prop = object_property_iter_next(&iter))) {
     ...
  }

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[AF: Fused ObjectPropertyIterator struct with typedef]
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/ppc/spapr_drc.c         |  7 +++----
 include/qom/object.h       | 30 ++++++++++++++----------------
 net/filter.c               |  7 +++----
 qmp.c                      | 14 ++++++--------
 qom/object.c               | 22 ++++------------------
 tests/check-qom-proplist.c |  7 +++----
 vl.c                       |  7 +++----
 7 files changed, 36 insertions(+), 58 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 4fb86a6..dccb908 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -684,7 +684,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
 {
     Object *root_container;
     ObjectProperty *prop;
-    ObjectPropertyIterator *iter;
+    ObjectPropertyIterator iter;
     uint32_t drc_count = 0;
     GArray *drc_indexes, *drc_power_domains;
     GString *drc_names, *drc_types;
@@ -708,8 +708,8 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
      */
     root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
 
-    iter = object_property_iter_init(root_container);
-    while ((prop = object_property_iter_next(iter))) {
+    object_property_iter_init(&iter, root_container);
+    while ((prop = object_property_iter_next(&iter))) {
         Object *obj;
         sPAPRDRConnector *drc;
         sPAPRDRConnectorClass *drck;
@@ -750,7 +750,6 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
                                     spapr_drc_get_type_str(drc->type));
         drc_types = g_string_insert_len(drc_types, -1, "\0", 1);
     }
-    object_property_iter_free(iter);
 
     /* now write the drc count into the space we reserved at the
      * beginning of the arrays previously
diff --git a/include/qom/object.h b/include/qom/object.h
index 37d414a..d0dafe9 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -966,7 +966,10 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
 ObjectProperty *object_class_property_find(ObjectClass *klass, const char *name,
                                            Error **errp);
 
-typedef struct ObjectPropertyIterator ObjectPropertyIterator;
+typedef struct ObjectPropertyIterator {
+    ObjectClass *nextclass;
+    GHashTableIter iter;
+} ObjectPropertyIterator;
 
 /**
  * object_property_iter_init:
@@ -984,32 +987,27 @@ typedef struct ObjectPropertyIterator ObjectPropertyIterator;
  *   <title>Using object property iterators</title>
  *   <programlisting>
  *   ObjectProperty *prop;
- *   ObjectPropertyIterator *iter;
+ *   ObjectPropertyIterator iter;
  *
- *   iter = object_property_iter_init(obj);
- *   while ((prop = object_property_iter_next(iter))) {
+ *   object_property_iter_init(&iter, obj);
+ *   while ((prop = object_property_iter_next(&iter))) {
  *     ... do something with prop ...
  *   }
- *   object_property_iter_free(iter);
  *   </programlisting>
  * </example>
- *
- * Returns: the new iterator
  */
-ObjectPropertyIterator *object_property_iter_init(Object *obj);
-
-/**
- * object_property_iter_free:
- * @iter: the iterator instance
- *
- * Releases any resources associated with the iterator.
- */
-void object_property_iter_free(ObjectPropertyIterator *iter);
+void object_property_iter_init(ObjectPropertyIterator *iter,
+                               Object *obj);
 
 /**
  * object_property_iter_next:
  * @iter: the iterator instance
  *
+ * Return the next available property. If no further properties
+ * are available, a %NULL value will be returned and the @iter
+ * pointer should not be used again after this point without
+ * re-initializing it.
+ *
  * Returns: the next property, or %NULL when all properties
  * have been traversed.
  */
diff --git a/net/filter.c b/net/filter.c
index f777ba2..5d90f83 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -137,7 +137,7 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
     Error *local_err = NULL;
     char *str, *info;
     ObjectProperty *prop;
-    ObjectPropertyIterator *iter;
+    ObjectPropertyIterator iter;
     StringOutputVisitor *ov;
 
     if (!nf->netdev_id) {
@@ -174,8 +174,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
     QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
 
     /* generate info str */
-    iter = object_property_iter_init(OBJECT(nf));
-    while ((prop = object_property_iter_next(iter))) {
+    object_property_iter_init(&iter, OBJECT(nf));
+    while ((prop = object_property_iter_next(&iter))) {
         if (!strcmp(prop->name, "type")) {
             continue;
         }
@@ -189,7 +189,6 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
         g_free(str);
         g_free(info);
     }
-    object_property_iter_free(iter);
 }
 
 static void netfilter_finalize(Object *obj)
diff --git a/qmp.c b/qmp.c
index 0a1fa19..3ff6db7 100644
--- a/qmp.c
+++ b/qmp.c
@@ -210,7 +210,7 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
     bool ambiguous = false;
     ObjectPropertyInfoList *props = NULL;
     ObjectProperty *prop;
-    ObjectPropertyIterator *iter;
+    ObjectPropertyIterator iter;
 
     obj = object_resolve_path(path, &ambiguous);
     if (obj == NULL) {
@@ -223,8 +223,8 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
         return NULL;
     }
 
-    iter = object_property_iter_init(obj);
-    while ((prop = object_property_iter_next(iter))) {
+    object_property_iter_init(&iter, obj);
+    while ((prop = object_property_iter_next(&iter))) {
         ObjectPropertyInfoList *entry = g_malloc0(sizeof(*entry));
 
         entry->value = g_malloc0(sizeof(ObjectPropertyInfo));
@@ -234,7 +234,6 @@ ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
         entry->value->name = g_strdup(prop->name);
         entry->value->type = g_strdup(prop->type);
     }
-    object_property_iter_free(iter);
 
     return props;
 }
@@ -506,7 +505,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
     ObjectClass *klass;
     Object *obj;
     ObjectProperty *prop;
-    ObjectPropertyIterator *iter;
+    ObjectPropertyIterator iter;
     DevicePropertyInfoList *prop_list = NULL;
 
     klass = object_class_by_name(typename);
@@ -535,8 +534,8 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
 
     obj = object_new(typename);
 
-    iter = object_property_iter_init(obj);
-    while ((prop = object_property_iter_next(iter))) {
+    object_property_iter_init(&iter, obj);
+    while ((prop = object_property_iter_next(&iter))) {
         DevicePropertyInfo *info;
         DevicePropertyInfoList *entry;
 
@@ -567,7 +566,6 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
         entry->next = prop_list;
         prop_list = entry;
     }
-    object_property_iter_free(iter);
 
     object_unref(obj);
 
diff --git a/qom/object.c b/qom/object.c
index 5d4c80b..5ff97ab 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -67,11 +67,6 @@ struct TypeImpl
     InterfaceImpl interfaces[MAX_INTERFACES];
 };
 
-struct ObjectPropertyIterator {
-    ObjectClass *nextclass;
-    GHashTableIter iter;
-};
-
 static Type type_interface;
 
 static GHashTable *type_table_get(void)
@@ -999,20 +994,11 @@ ObjectProperty *object_property_find(Object *obj, const char *name,
     return NULL;
 }
 
-ObjectPropertyIterator *object_property_iter_init(Object *obj)
+void object_property_iter_init(ObjectPropertyIterator *iter,
+                               Object *obj)
 {
-    ObjectPropertyIterator *ret = g_new0(ObjectPropertyIterator, 1);
-    g_hash_table_iter_init(&ret->iter, obj->properties);
-    ret->nextclass = object_get_class(obj);
-    return ret;
-}
-
-void object_property_iter_free(ObjectPropertyIterator *iter)
-{
-    if (!iter) {
-        return;
-    }
-    g_free(iter);
+    g_hash_table_iter_init(&iter->iter, obj->properties);
+    iter->nextclass = object_get_class(obj);
 }
 
 ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter)
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 5167e78..448d270 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -455,11 +455,11 @@ static void test_dummy_iterator(void)
                               NULL));
 
     ObjectProperty *prop;
-    ObjectPropertyIterator *iter;
+    ObjectPropertyIterator iter;
     bool seenbv = false, seensv = false, seenav = false, seentype;
 
-    iter = object_property_iter_init(OBJECT(dobj));
-    while ((prop = object_property_iter_next(iter))) {
+    object_property_iter_init(&iter, OBJECT(dobj));
+    while ((prop = object_property_iter_next(&iter))) {
         if (g_str_equal(prop->name, "bv")) {
             seenbv = true;
         } else if (g_str_equal(prop->name, "sv")) {
@@ -474,7 +474,6 @@ static void test_dummy_iterator(void)
             g_assert_not_reached();
         }
     }
-    object_property_iter_free(iter);
     g_assert(seenbv);
     g_assert(seenav);
     g_assert(seensv);
diff --git a/vl.c b/vl.c
index b7a083e..f043009 100644
--- a/vl.c
+++ b/vl.c
@@ -1535,14 +1535,14 @@ MachineInfoList *qmp_query_machines(Error **errp)
 static int machine_help_func(QemuOpts *opts, MachineState *machine)
 {
     ObjectProperty *prop;
-    ObjectPropertyIterator *iter;
+    ObjectPropertyIterator iter;
 
     if (!qemu_opt_has_help_opt(opts)) {
         return 0;
     }
 
-    iter = object_property_iter_init(OBJECT(machine));
-    while ((prop = object_property_iter_next(iter))) {
+    object_property_iter_init(&iter, OBJECT(machine));
+    while ((prop = object_property_iter_next(&iter))) {
         if (!prop->set) {
             continue;
         }
@@ -1555,7 +1555,6 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine)
             error_printf("\n");
         }
     }
-    object_property_iter_free(iter);
 
     return 1;
 }
-- 
2.6.2