Patchwork [RESEND,1/2] qapi: introduce strList and visit_type_strList()

login
register
mail settings
Submitter Amos Kong
Date April 24, 2013, 12:47 p.m.
Message ID <1366807646-8473-1-git-send-email-akong@redhat.com>
Download mbox | patch
Permalink /patch/239194/
State New
Headers show

Comments

Amos Kong - April 24, 2013, 12:47 p.m.
Currently we can only use ['String'] to add string to a list,
it contains some additional JSON structure.
    "multicast": [
        {
            "str": "01:80:c2:00:00:21"
        },
        {
            "str": "00:00:00:00:00:00"
        }
    ]

This patch introdued strList, we can use ['str']

    "multicast": [
        "01:00:5e:00:00:01",
        "33:33:ff:12:34:57"
    ]

Signed-off-by: Amos Kong <akong@redhat.com>
---
I used an _ugly_ "#ifndef" to limit it only be defined in qapi-types.h.
do you have some suggestion to fix it?
---
 include/qapi/visitor.h |    2 ++
 qapi/qapi-visit-core.c |   23 +++++++++++++++++++++++
 scripts/qapi-types.py  |    8 ++++++++
 3 files changed, 33 insertions(+), 0 deletions(-)
Eric Blake - April 24, 2013, 2:01 p.m.
On 04/24/2013 06:47 AM, Amos Kong wrote:
> Currently we can only use ['String'] to add string to a list,
> it contains some additional JSON structure.
>     "multicast": [
>         {
>             "str": "01:80:c2:00:00:21"
>         },
>         {
>             "str": "00:00:00:00:00:00"
>         }
>     ]
> 
> This patch introdued strList, we can use ['str']
> 
>     "multicast": [
>         "01:00:5e:00:00:01",
>         "33:33:ff:12:34:57"
>     ]
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> I used an _ugly_ "#ifndef" to limit it only be defined in qapi-types.h.
> do you have some suggestion to fix it?

I'm not the maintainer, so you may still want to wait for a better
suggestion.  But since I personally don't mind the #ifdef if that's what
it takes to get this to compile, and since I'd like to see the feature
added, I'm fine giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

That said, I think the API in patch 2/2 is not quite right; and when you
fix things there, you may not need this patch in the 1.5 timeframe...
Eric Blake - April 24, 2013, 2:39 p.m.
On 04/24/2013 06:47 AM, Amos Kong wrote:
> Currently we can only use ['String'] to add string to a list,
> it contains some additional JSON structure.

Side note - when sending a 2-patch series, it helps to include a 0/2
cover letter.
Amos Kong - April 24, 2013, 3:59 p.m.
On Wed, Apr 24, 2013 at 08:01:50AM -0600, Eric Blake wrote:
> On 04/24/2013 06:47 AM, Amos Kong wrote:
> > Currently we can only use ['String'] to add string to a list,
> > it contains some additional JSON structure.
> >     "multicast": [
> >         {
> >             "str": "01:80:c2:00:00:21"
> >         },
> >         {
> >             "str": "00:00:00:00:00:00"
> >         }
> >     ]
> > 
> > This patch introdued strList, we can use ['str']
> > 
> >     "multicast": [
> >         "01:00:5e:00:00:01",
> >         "33:33:ff:12:34:57"
> >     ]
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> > I used an _ugly_ "#ifndef" to limit it only be defined in qapi-types.h.
> > do you have some suggestion to fix it?
> 
> I'm not the maintainer, so you may still want to wait for a better
> suggestion.  But since I personally don't mind the #ifdef if that's what
> it takes to get this to compile, and since I'd like to see the feature
> added, I'm fine giving:
 
I will send a v2 to solve this issue.

> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> That said, I think the API in patch 2/2 is not quite right; and when you
> fix things there, you may not need this patch in the 1.5 timeframe...

I already saw your comment, will split those two patches.
Thanks

Patch

diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1fef18c..a79ce79 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -50,6 +50,8 @@  void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
 void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
+void visit_type_strList(Visitor *m, strList ** obj, const char *name,
+                        Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 401ee6e..dc54cc8 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -257,6 +257,29 @@  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
     }
 }
 
+void visit_type_strList(Visitor *m, strList ** obj, const char *name,
+                        Error **errp)
+{
+    GenericList *i, **prev = (GenericList **)obj;
+    Error *err = NULL;
+
+    if (!error_is_set(errp)) {
+        visit_start_list(m, name, &err);
+        if (!err) {
+            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
+                strList *native_i = (strList *)i;
+                visit_type_str(m, &native_i->value, NULL, &err);
+            }
+            error_propagate(errp, err);
+            err = NULL;
+
+            /* Always call end_list if start_list succeeded.  */
+            visit_end_list(m, &err);
+        }
+        error_propagate(errp, err);
+    }
+}
+
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
 {
     if (!error_is_set(errp)) {
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9e19920..f2ca373 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -276,6 +276,14 @@  fdecl.write(mcgen('''
 #include <stdbool.h>
 #include <stdint.h>
 
+#ifndef QGA_QAPI_TYPES_H
+typedef struct strList
+{
+  char *value;
+  struct strList *next;
+} strList;
+#endif
+
 ''',
                   guard=guardname(h_file)))