diff mbox

[v7,13/31] qapi: Drop unused 'kind' for struct/enum visit

Message ID 1449546921-6378-14-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Dec. 8, 2015, 3:55 a.m. UTC 
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 <eblake@redhat.com>

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

Comments

David Gibson Dec. 8, 2015, 4:40 a.m. UTC | #1 
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 <eblake@redhat.com>

For spapr parts:

Acked-by: David Gibson <david@gibson.dropbear.id.au>
Eric Blake Dec. 11, 2015, 1:51 p.m. UTC | #2 
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.
diff mbox

Patch

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, &err);
+    visit_start_struct(v, NULL, 0, NULL, &err);
     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, &err);
+        visit_start_struct(v, NULL, 0, name, &err);
         if (!err) {
             visit_end_struct(v, &err);
         }
@@ -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, &name_len);
-            visit_start_struct(v, NULL, NULL, name, 0, &err);
+            visit_start_struct(v, NULL, 0, name, &err);
             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, &err);
+    visit_start_struct(v, NULL, 0, name, &err);
     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, &err);
+    visit_start_struct(v, NULL, 0, "stats", &err);
     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 *name, Error **errp);
     void (*end_struct)(Visitor *v, Error **errp);

     void (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
@@ -30,8 +30,8 @@  struct Visitor
     GenericList *(*next_list)(Visitor *v, GenericList **list, Error **errp);
     void (*end_list)(Visitor *v, Error **errp);

-    void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
-                      const char *kind, const char *name, Error **errp);
+    void (*type_enum)(Visitor *v, int *obj, const char *const strings[],
+                      const char *name, Error **errp);
     /* May be NULL; only needed for input visitors. */
     void (*get_next_type)(Visitor *v, QType *type, bool promote_int,
                           const char *name, Error **errp);
@@ -60,9 +60,9 @@  struct Visitor
     void (*end_union)(Visitor *v, bool data_present, Error **errp);
 };

-void input_type_enum(Visitor *v, int *obj, const char * const strings[],
-                     const char *kind, const char *name, Error **errp);
-void output_type_enum(Visitor *v, int *obj, const char * const strings[],
-                      const char *kind, const char *name, Error **errp);
+void input_type_enum(Visitor *v, int *obj, const char *const strings[],
+                     const char *name, Error **errp);
+void output_type_enum(Visitor *v, int *obj, const char *const strings[],
+                      const char *name, Error **errp);

 #endif
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index a14a16d..21891ca 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -27,8 +27,8 @@  typedef struct GenericList
     struct GenericList *next;
 } GenericList;

-void visit_start_struct(Visitor *v, void **obj, const char *kind,
-                        const char *name, size_t size, Error **errp);
+void visit_start_struct(Visitor *v, void **obj, size_t size,
+                        const char *name, Error **errp);
 void visit_end_struct(Visitor *v, Error **errp);
 void visit_start_implicit_struct(Visitor *v, void **obj, size_t size,
                                  Error **errp);
@@ -53,8 +53,8 @@  bool visit_optional(Visitor *v, bool *present, const char *name);
  */
 void visit_get_next_type(Visitor *v, QType *type, bool promote_int,
                          const char *name, Error **errp);
-void visit_type_enum(Visitor *v, int *obj, const char * const strings[],
-                     const char *kind, const char *name, Error **errp);
+void visit_type_enum(Visitor *v, int *obj, const char *const strings[],
+                     const char *name, Error **errp);
 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp);
 void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp);
 void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp);
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 56c798f..df5b537 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -124,8 +124,8 @@  opts_visitor_insert(GHashTable *unprocessed_opts, const QemuOpt *opt)


 static void
-opts_start_struct(Visitor *v, void **obj, const char *kind,
-                  const char *name, size_t size, Error **errp)
+opts_start_struct(Visitor *v, void **obj, size_t size,
+                  const char *name, Error **errp)
 {
     OptsVisitor *ov = to_ov(v);
     const QemuOpt *opt;
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 11eb828..f4c0a67 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -58,9 +58,8 @@  static void *qapi_dealloc_pop(QapiDeallocVisitor *qov)
     return value;
 }

-static void qapi_dealloc_start_struct(Visitor *v, void **obj, const char *kind,
-                                      const char *name, size_t unused,
-                                      Error **errp)
+static void qapi_dealloc_start_struct(Visitor *v, void **obj, size_t unused,
+                                      const char *name, Error **errp)
 {
     QapiDeallocVisitor *qov = to_qov(v);
     qapi_dealloc_push(qov, obj);
@@ -164,9 +163,8 @@  static void qapi_dealloc_type_anything(Visitor *v, QObject **obj,
 }

 static void qapi_dealloc_type_enum(Visitor *v, int *obj,
-                                   const char * const strings[],
-                                   const char *kind, const char *name,
-                                   Error **errp)
+                                   const char *const strings[],
+                                   const char *name, Error **errp)
 {
 }

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index a48fd4e..d9ec1cc 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -2,6 +2,7 @@ 
  * Core Definitions for QAPI Visitor Classes
  *
  * Copyright IBM, Corp. 2011
+ * Copyright (C) 2015 Red Hat, Inc.
  *
  * Authors:
  *  Anthony Liguori   <aliguori@us.ibm.com>
@@ -17,10 +18,10 @@ 
 #include "qapi/visitor.h"
 #include "qapi/visitor-impl.h"

-void visit_start_struct(Visitor *v, void **obj, const char *kind,
-                        const char *name, size_t size, Error **errp)
+void visit_start_struct(Visitor *v, void **obj, size_t size,
+                        const char *name, Error **errp)
 {
-    v->start_struct(v, obj, kind, name, size, errp);
+    v->start_struct(v, obj, size, name, errp);
 }

 void visit_end_struct(Visitor *v, Error **errp)
@@ -89,10 +90,10 @@  void visit_get_next_type(Visitor *v, QType *type, bool promote_int,
     }
 }

-void visit_type_enum(Visitor *v, int *obj, const char * const strings[],
-                     const char *kind, const char *name, Error **errp)
+void visit_type_enum(Visitor *v, int *obj, const char *const strings[],
+                     const char *name, Error **errp)
 {
-    v->type_enum(v, obj, strings, kind, name, errp);
+    v->type_enum(v, obj, strings, name, errp);
 }

 void visit_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
@@ -220,9 +221,8 @@  void visit_type_any(Visitor *v, QObject **obj, const char *name,
     v->type_any(v, obj, name, errp);
 }

-void output_type_enum(Visitor *v, int *obj, const char * const strings[],
-                      const char *kind, const char *name,
-                      Error **errp)
+void output_type_enum(Visitor *v, int *obj, const char *const strings[],
+                      const char *name, Error **errp)
 {
     int i = 0;
     int value = *obj;
@@ -239,9 +239,8 @@  void output_type_enum(Visitor *v, int *obj, const char * const strings[],
     visit_type_str(v, &enum_str, name, errp);
 }

-void input_type_enum(Visitor *v, int *obj, const char * const strings[],
-                     const char *kind, const char *name,
-                     Error **errp)
+void input_type_enum(Visitor *v, int *obj, const char *const strings[],
+                     const char *name, Error **errp)
 {
     Error *local_err = NULL;
     int64_t value = 0;
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 32b60bb..45ddce7 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -114,8 +114,8 @@  static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
     qiv->nb_stack--;
 }

-static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
-                                   const char *name, size_t size, Error **errp)
+static void qmp_input_start_struct(Visitor *v, void **obj, size_t size,
+                                   const char *name, Error **errp)
 {
     QmpInputVisitor *qiv = to_qiv(v);
     QObject *qobj = qmp_input_get_object(qiv, name, true);
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index f8eebaa..8cb82cb 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -110,9 +110,8 @@  static void qmp_output_add_obj(QmpOutputVisitor *qov, const char *name,
     }
 }

-static void qmp_output_start_struct(Visitor *v, void **obj, const char *kind,
-                                    const char *name, size_t unused,
-                                    Error **errp)
+static void qmp_output_start_struct(Visitor *v, void **obj, size_t unused,
+                                    const char *name, Error **errp)
 {
     QmpOutputVisitor *qov = to_qov(v);
     QDict *dict = qdict_new();
diff --git a/qom/object.c b/qom/object.c
index d751569..5eb65ea 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1186,7 +1186,7 @@  int object_property_get_enum(Object *obj, const char *name,
     siv = string_input_visitor_new(str);
     string_output_visitor_cleanup(sov);
     visit_type_enum(string_input_get_visitor(siv),
-                    &ret, enumprop->strings, NULL, name, errp);
+                    &ret, enumprop->strings, name, errp);

     g_free(str);
     string_input_visitor_cleanup(siv);
@@ -1810,7 +1810,7 @@  static void property_get_enum(Object *obj, Visitor *v, void *opaque,
         return;
     }

-    visit_type_enum(v, &value, prop->strings, NULL, name, errp);
+    visit_type_enum(v, &value, prop->strings, name, errp);
 }

 static void property_set_enum(Object *obj, Visitor *v, void *opaque,
@@ -1820,7 +1820,7 @@  static void property_set_enum(Object *obj, Visitor *v, void *opaque,
     int value;
     Error *err = NULL;

-    visit_type_enum(v, &value, prop->strings, NULL, name, &err);
+    visit_type_enum(v, &value, prop->strings, name, &err);
     if (err) {
         error_propagate(errp, err);
         return;
@@ -1876,7 +1876,7 @@  static void property_get_tm(Object *obj, Visitor *v, void *opaque,
         goto out;
     }

-    visit_start_struct(v, NULL, "struct tm", name, 0, &err);
+    visit_start_struct(v, NULL, 0, name, &err);
     if (err) {
         goto out;
     }
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index e37c07a..111958d 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -63,7 +63,7 @@  def gen_event_send(name, arg_type):
     qov = qmp_output_visitor_new();
     v = qmp_output_get_visitor(qov);

-    visit_start_struct(v, NULL, NULL, "%(name)s", 0, &err);
+    visit_start_struct(v, NULL, 0, "%(name)s", &err);
 ''',
                      name=name)
         ret += gen_err_check()
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 6bd188b..1a86afd 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -122,7 +122,7 @@  void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
 {
     Error *err = NULL;

-    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+    visit_start_struct(v, (void **)obj, sizeof(%(c_name)s), name, &err);
     if (err) {
         goto out;
     }
@@ -138,7 +138,7 @@  out:
     error_propagate(errp, err);
 }
 ''',
-                 name=name, c_name=c_name(name))
+                 c_name=c_name(name))

     return ret

@@ -183,11 +183,11 @@  def gen_visit_enum(name):
 void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error **errp)
 {
     int tmp = *obj;
-    visit_type_enum(v, &tmp, %(c_name)s_lookup, "%(name)s", name, errp);
+    visit_type_enum(v, &tmp, %(c_name)s_lookup, name, errp);
     *obj = tmp;
 }
 ''',
-                 c_name=c_name(name), name=name)
+                 c_name=c_name(name))


 def gen_visit_alternate(name, variants):
@@ -259,7 +259,7 @@  void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
 {
     Error *err = NULL;

-    visit_start_struct(v, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
+    visit_start_struct(v, (void **)obj, sizeof(%(c_name)s), name, &err);
     if (err) {
         goto out;
     }
@@ -267,7 +267,7 @@  void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
         goto out_obj;
     }
 ''',
-                 c_name=c_name(name), name=name)
+                 c_name=c_name(name))

     if base:
         ret += mcgen('''
diff --git a/vl.c b/vl.c
index 11555ac..1010bdb 100644
--- a/vl.c
+++ b/vl.c
@@ -2840,7 +2840,7 @@  static int object_create(void *opaque, QemuOpts *opts, Error **errp)
     pdict = qemu_opts_to_qdict(opts, NULL);
     v = opts_get_visitor(ov);

-    visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+    visit_start_struct(v, NULL, 0, NULL, &err);
     if (err) {
         goto out;
     }