Re: [Qemu-devel] [PATCH v7 13/31] qapi: Drop unused 'kind' for struct/enum visit

2015-12-11 Thread Eric Blake 
On 12/07/2015 08:55 PM, Eric Blake wrote:
> visit_start_struct() and visit_type_enum() had a 'kind' argument
> that was usually set to either the stringized version of the
> corresponding qapi type name, or to NULL (although some clients
> didn't even get that right).  But nothing ever used the argument.
> It's even hard to argue that it would be useful in a debugger,
> as a stack backtrace also tells which type is being visited.
> 
> Therefore, drop the 'kind' argument as dead.  While at it, change
> the signature of visit_start_struct() to place the 'name'
> argument at the end (other than 'errp'), and the 'size' argument
> next to 'obj'; this placement of 'name' matches matches how all
> other functions in visit.h do it (visit_type_enum() places
> 'strings' between 'obj' and 'name'; visit_get_next_type() places
> 'promote_int' between 'type' and 'name').  This also avoids the
> confusion caused by splitting related pieces of information,
> where the old signature an unrelated parameter in between the
> "typename" and sizeof(typename) arguments.

I should probably spell it out better in the commit message; I was going
from:

visit_start_struct(v, obj, [kind,] name, size, err)

to:

visit_start_struct(v, obj, size, [kind,] name, err)

then dropping kind as unused.  But I'm seriously thinking about doing
the argument shuffle in the opposite direction (move name earlier,
rather than later):

visit_start_struct(v, name, obj, [kind,] size, err)

with a global coccinelle patch that changes ALL visit_type_* to put name
before obj, because after all we are tying this to JSON which uses
"name":value and it looks odd that every one of our visitor functions
takes parameters in 'value, name' order.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v7 13/31] qapi: Drop unused 'kind' for struct/enum visit

2015-12-08 Thread David Gibson 
On Mon, Dec 07, 2015 at 08:55:03PM -0700, Eric Blake wrote:
> visit_start_struct() and visit_type_enum() had a 'kind' argument
> that was usually set to either the stringized version of the
> corresponding qapi type name, or to NULL (although some clients
> didn't even get that right).  But nothing ever used the argument.
> It's even hard to argue that it would be useful in a debugger,
> as a stack backtrace also tells which type is being visited.
> 
> Therefore, drop the 'kind' argument as dead.  While at it, change
> the signature of visit_start_struct() to place the 'name'
> argument at the end (other than 'errp'), and the 'size' argument
> next to 'obj'; this placement of 'name' matches matches how all
> other functions in visit.h do it (visit_type_enum() places
> 'strings' between 'obj' and 'name'; visit_get_next_type() places
> 'promote_int' between 'type' and 'name').  This also avoids the
> confusion caused by splitting related pieces of information,
> where the old signature an unrelated parameter in between the
> "typename" and sizeof(typename) arguments.
> 
> Signed-off-by: Eric Blake 

For spapr parts:

Acked-by: David Gibson 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v7 13/31] qapi: Drop unused 'kind' for struct/enum visit

2015-12-07 Thread Eric Blake 
visit_start_struct() and visit_type_enum() had a 'kind' argument
that was usually set to either the stringized version of the
corresponding qapi type name, or to NULL (although some clients
didn't even get that right).  But nothing ever used the argument.
It's even hard to argue that it would be useful in a debugger,
as a stack backtrace also tells which type is being visited.

Therefore, drop the 'kind' argument as dead.  While at it, change
the signature of visit_start_struct() to place the 'name'
argument at the end (other than 'errp'), and the 'size' argument
next to 'obj'; this placement of 'name' matches matches how all
other functions in visit.h do it (visit_type_enum() places
'strings' between 'obj' and 'name'; visit_get_next_type() places
'promote_int' between 'type' and 'name').  This also avoids the
confusion caused by splitting related pieces of information,
where the old signature an unrelated parameter in between the
"typename" and sizeof(typename) arguments.

Signed-off-by: Eric Blake 

---
v7: new patch
---
 hmp.c   |  2 +-
 hw/core/qdev-properties.c   |  6 ++
 hw/ppc/spapr_drc.c  |  4 ++--
 hw/virtio/virtio-balloon.c  |  4 ++--
 include/qapi/visitor-impl.h | 16 
 include/qapi/visitor.h  |  8 
 qapi/opts-visitor.c |  4 ++--
 qapi/qapi-dealloc-visitor.c | 10 --
 qapi/qapi-visit-core.c  | 23 +++
 qapi/qmp-input-visitor.c|  4 ++--
 qapi/qmp-output-visitor.c   |  5 ++---
 qom/object.c|  8 
 scripts/qapi-event.py   |  2 +-
 scripts/qapi-visit.py   | 12 ++--
 vl.c|  2 +-
 15 files changed, 52 insertions(+), 58 deletions(-)

diff --git a/hmp.c b/hmp.c
index 0d21f1d..f81f332 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1680,7 +1680,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
 pdict = qdict_clone_shallow(qdict);
 v = opts_get_visitor(ov);

-visit_start_struct(v, NULL, NULL, NULL, 0, );
+visit_start_struct(v, NULL, 0, NULL, );
 if (err) {
 goto out_clean;
 }
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 33e245e..d81d689 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -48,8 +48,7 @@ static void get_enum(Object *obj, Visitor *v, void *opaque,
 Property *prop = opaque;
 int *ptr = qdev_get_prop_ptr(dev, prop);

-visit_type_enum(v, ptr, prop->info->enum_table,
-prop->info->name, prop->name, errp);
+visit_type_enum(v, ptr, prop->info->enum_table, prop->name, errp);
 }

 static void set_enum(Object *obj, Visitor *v, void *opaque,
@@ -64,8 +63,7 @@ static void set_enum(Object *obj, Visitor *v, void *opaque,
 return;
 }

-visit_type_enum(v, ptr, prop->info->enum_table,
-prop->info->name, prop->name, errp);
+visit_type_enum(v, ptr, prop->info->enum_table, prop->name, errp);
 }

 /* Bit */
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 8be62c3..96d06f5 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -259,7 +259,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, void 
*opaque,
 void *fdt;

 if (!drc->fdt) {
-visit_start_struct(v, NULL, NULL, name, 0, );
+visit_start_struct(v, NULL, 0, name, );
 if (!err) {
 visit_end_struct(v, );
 }
@@ -282,7 +282,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, void 
*opaque,
 case FDT_BEGIN_NODE:
 fdt_depth++;
 name = fdt_get_name(fdt, fdt_offset, _len);
-visit_start_struct(v, NULL, NULL, name, 0, );
+visit_start_struct(v, NULL, 0, name, );
 if (err) {
 error_propagate(errp, err);
 return;
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1ce987a..cb8237f 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -117,7 +117,7 @@ static void balloon_stats_get_all(Object *obj, struct 
Visitor *v,
 VirtIOBalloon *s = opaque;
 int i;

-visit_start_struct(v, NULL, "guest-stats", name, 0, );
+visit_start_struct(v, NULL, 0, name, );
 if (err) {
 goto out;
 }
@@ -126,7 +126,7 @@ static void balloon_stats_get_all(Object *obj, struct 
Visitor *v,
 goto out_end;
 }

-visit_start_struct(v, NULL, NULL, "stats", 0, );
+visit_start_struct(v, NULL, 0, "stats", );
 if (err) {
 goto out_end;
 }
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 5ee2974..6737005 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -18,8 +18,8 @@
 struct Visitor
 {
 /* Must be set */
-void (*start_struct)(Visitor *v, void **obj, const char *kind,
- const char *name, size_t size, Error **errp);
+void (*start_struct)(Visitor *v, void **obj, size_t size,
+ const char