* [Qemu-devel] [PATCH] qom: change object property iterator API contract
@ 2015-11-27 15:27 Daniel P. Berrange
2015-12-08 16:05 ` Eric Blake
0 siblings, 1 reply; 3+ messages in thread
From: Daniel P. Berrange @ 2015-11-27 15:27 UTC (permalink / raw)
To: qemu-devel; +Cc: Andreas Färber, Markus Armbruster
Currently the object property 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))) {
...
}
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
NB, this patch is not against master, it is intended to apply
after
"qom: allow properties to be registered against classes"
which is queued in qom-next for 2.6
hw/ppc/spapr_drc.c | 7 +++----
include/qom/object.h | 42 +++++++++++++++++++++++++++---------------
net/filter.c | 7 +++----
qmp.c | 14 ++++++--------
qom/object.c | 22 ++++------------------
tests/check-qom-proplist.c | 7 +++----
vl.c | 7 +++----
7 files changed, 49 insertions(+), 57 deletions(-)
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index f34bc04..2ef69e5 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -657,7 +657,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;
@@ -681,8 +681,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;
@@ -723,7 +723,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 998e322..c06182b 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -346,6 +346,7 @@ typedef struct ObjectProperty
void *opaque;
} ObjectProperty;
+
/**
* ObjectUnparent:
* @obj: the object that is being removed from the composition tree
@@ -971,6 +972,11 @@ ObjectProperty *object_class_property_find(ObjectClass *klass, const char *name,
typedef struct ObjectPropertyIterator ObjectPropertyIterator;
+struct ObjectPropertyIterator {
+ ObjectClass *nextclass;
+ GHashTableIter iter;
+};
+
/**
* object_property_iter_init:
* @obj: the object
@@ -988,37 +994,43 @@ 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.
*/
ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter);
+ /**
+ * object_property_iter_free:
+ * @iter: the iterator instance
+ *
+ * Releases any resources associated with the iterator. It is
+ * not necessary to call this method if object_property_iter_next
+ * has returned %NULL. It is only required if an application wishes
+ * to abort iteration before it is complete
+ */
+void object_property_iter_free(ObjectPropertyIterator *iter);
+
void object_unparent(Object *obj);
/**
diff --git a/net/filter.c b/net/filter.c
index 1365bad..b820ab0 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 c21ab00..c77bf41 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 4211ff1..20b3f1c 100644
--- a/vl.c
+++ b/vl.c
@@ -1536,14 +1536,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;
}
@@ -1556,7 +1556,6 @@ static int machine_help_func(QemuOpts *opts, MachineState *machine)
error_printf("\n");
}
}
- object_property_iter_free(iter);
return 1;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] qom: change object property iterator API contract
2015-11-27 15:27 [Qemu-devel] [PATCH] qom: change object property iterator API contract Daniel P. Berrange
@ 2015-12-08 16:05 ` Eric Blake
2015-12-09 12:29 ` Daniel P. Berrange
0 siblings, 1 reply; 3+ messages in thread
From: Eric Blake @ 2015-12-08 16:05 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Andreas Färber, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]
On 11/27/2015 08:27 AM, Daniel P. Berrange wrote:
> Currently the object property 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))) {
> ...
> }
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>
> NB, this patch is not against master, it is intended to apply
> after
>
> "qom: allow properties to be registered against classes"
>
> which is queued in qom-next for 2.6
>
> hw/ppc/spapr_drc.c | 7 +++----
> include/qom/object.h | 42 +++++++++++++++++++++++++++---------------
> net/filter.c | 7 +++----
> qmp.c | 14 ++++++--------
> qom/object.c | 22 ++++------------------
> tests/check-qom-proplist.c | 7 +++----
> vl.c | 7 +++----
> 7 files changed, 49 insertions(+), 57 deletions(-)
>
> +++ b/include/qom/object.h
> @@ -346,6 +346,7 @@ typedef struct ObjectProperty
> void *opaque;
> } ObjectProperty;
>
> +
> /**
> * ObjectUnparent:
> * @obj: the object that is being removed from the composition tree
Spurious whitespace change?
>
> + /**
> + * object_property_iter_free:
> + * @iter: the iterator instance
> + *
> + * Releases any resources associated with the iterator. It is
> + * not necessary to call this method if object_property_iter_next
> + * has returned %NULL. It is only required if an application wishes
> + * to abort iteration before it is complete
> + */
> +void object_property_iter_free(ObjectPropertyIterator *iter);
> +
Huh? Why is this being added? I thought the point was to get rid of the
need for object_property_iter_free().
> +++ b/qom/object.c
> @@ -67,11 +67,6 @@ struct TypeImpl
Other than that snafu, everything else looked fine. If that's all you
fix for v2, you can add:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] qom: change object property iterator API contract
2015-12-08 16:05 ` Eric Blake
@ 2015-12-09 12:29 ` Daniel P. Berrange
0 siblings, 0 replies; 3+ messages in thread
From: Daniel P. Berrange @ 2015-12-09 12:29 UTC (permalink / raw)
To: Eric Blake; +Cc: Andreas Färber, qemu-devel, Markus Armbruster
On Tue, Dec 08, 2015 at 09:05:52AM -0700, Eric Blake wrote:
> On 11/27/2015 08:27 AM, Daniel P. Berrange wrote:
> > Currently the object property 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))) {
> > ...
> > }
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >
> > NB, this patch is not against master, it is intended to apply
> > after
> >
> > "qom: allow properties to be registered against classes"
> >
> > which is queued in qom-next for 2.6
> >
> > hw/ppc/spapr_drc.c | 7 +++----
> > include/qom/object.h | 42 +++++++++++++++++++++++++++---------------
> > net/filter.c | 7 +++----
> > qmp.c | 14 ++++++--------
> > qom/object.c | 22 ++++------------------
> > tests/check-qom-proplist.c | 7 +++----
> > vl.c | 7 +++----
> > 7 files changed, 49 insertions(+), 57 deletions(-)
> >
>
> > +++ b/include/qom/object.h
> > @@ -346,6 +346,7 @@ typedef struct ObjectProperty
> > void *opaque;
> > } ObjectProperty;
> >
> > +
> > /**
> > * ObjectUnparent:
> > * @obj: the object that is being removed from the composition tree
>
> Spurious whitespace change?
>
>
> >
> > + /**
> > + * object_property_iter_free:
> > + * @iter: the iterator instance
> > + *
> > + * Releases any resources associated with the iterator. It is
> > + * not necessary to call this method if object_property_iter_next
> > + * has returned %NULL. It is only required if an application wishes
> > + * to abort iteration before it is complete
> > + */
> > +void object_property_iter_free(ObjectPropertyIterator *iter);
> > +
>
> Huh? Why is this being added? I thought the point was to get rid of the
> need for object_property_iter_free().
Yeh, forgot to remove the header declaration.
>
> > +++ b/qom/object.c
> > @@ -67,11 +67,6 @@ struct TypeImpl
>
> Other than that snafu, everything else looked fine. If that's all you
> fix for v2, you can add:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Ok, thanks for the review
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-09 12:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-27 15:27 [Qemu-devel] [PATCH] qom: change object property iterator API contract Daniel P. Berrange
2015-12-08 16:05 ` Eric Blake
2015-12-09 12:29 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).