Re: [Qemu-devel] [PATCH v2] qom: change object property iterator API contract

2016-01-11 Thread Andreas Färber
Am 17.12.2015 um 09:54 schrieb Markus Armbruster:
> "Daniel P. Berrange"  writes:
> 
>> 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))) {
>>  ...
>>   }
>>
>> Reviewed-by: Eric Blake 
>> Signed-off-by: Daniel P. Berrange 
> [...]
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index 9630c5b..275a21b 100644
>> --- a/include/qom/object.h
>> +++ b/include/qom/object.h
>> @@ -971,6 +971,11 @@ ObjectProperty *object_class_property_find(ObjectClass 
>> *klass, const char *name,
>>  
>>  typedef struct ObjectPropertyIterator ObjectPropertyIterator;
>>  
>> +struct ObjectPropertyIterator {
>> +ObjectClass *nextclass;
>> +GHashTableIter iter;
>> +};
>> +
> 
> Please fuse the two declarations.  Perhaps Andreas can do that on
> commit.

Done.

>>  /**
>>   * object_property_iter_init:
>>   * @obj: the object
> 
> Reviewed-by: Markus Armbruster 

Queued on qom-next:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Thanks,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)



Re: [Qemu-devel] [PATCH v2] qom: change object property iterator API contract

2015-12-17 Thread Markus Armbruster
"Daniel P. Berrange"  writes:

> 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))) {
>  ...
>   }
>
> Reviewed-by: Eric Blake 
> Signed-off-by: Daniel P. Berrange 
[...]
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 9630c5b..275a21b 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -971,6 +971,11 @@ ObjectProperty *object_class_property_find(ObjectClass 
> *klass, const char *name,
>  
>  typedef struct ObjectPropertyIterator ObjectPropertyIterator;
>  
> +struct ObjectPropertyIterator {
> +ObjectClass *nextclass;
> +GHashTableIter iter;
> +};
> +

Please fuse the two declarations.  Perhaps Andreas can do that on
commit.

>  /**
>   * object_property_iter_init:
>   * @obj: the object

Reviewed-by: Markus Armbruster 



[Qemu-devel] [PATCH v2] qom: change object property iterator API contract

2015-12-09 Thread Daniel P. Berrange
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))) {
 ...
  }

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---

Changed in v2:

 - Remove accidentally added object_property_iter_free
   header declaration
 - Remove accidental whitespace change in header

 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, 37 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 9630c5b..275a21b 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -971,6 +971,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
@@ -987,32 +992,27 @@ typedef struct ObjectPropertyIterator 
ObjectPropertyIterator;
  *   Using object property iterators
  *   
  *   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);
  *   
  * 
- *
- * 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 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_in