From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36965) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9G6s-0003N6-2W for qemu-devel@nongnu.org; Thu, 29 Sep 2011 08:53:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R9G6q-0008H6-IH for qemu-devel@nongnu.org; Thu, 29 Sep 2011 08:53:06 -0400 Received: from e8.ny.us.ibm.com ([32.97.182.138]:59629) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R9G6q-0008Gz-D3 for qemu-devel@nongnu.org; Thu, 29 Sep 2011 08:53:04 -0400 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by e8.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p8TCc6bj008864 for ; Thu, 29 Sep 2011 08:38:06 -0400 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p8TCr3J23268672 for ; Thu, 29 Sep 2011 08:53:03 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p8TCr2wG012698 for ; Thu, 29 Sep 2011 08:53:03 -0400 Message-ID: <4E846A2C.5080203@us.ibm.com> Date: Thu, 29 Sep 2011 07:53:00 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1317221085-5825-1-git-send-email-lcapitulino@redhat.com> <1317221085-5825-11-git-send-email-lcapitulino@redhat.com> In-Reply-To: <1317221085-5825-11-git-send-email-lcapitulino@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/21] qapi: modify visitor code generation for list iteration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: kwolf@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On 09/28/2011 09:44 AM, Luiz Capitulino wrote: > From: Michael Roth > > Modify logic such that we never assign values to the list head argument > to progress through the list on subsequent iterations, instead rely only > on having our return value passed back in as an argument on the next > call. Also update QMP I/O visitors and test cases accordingly, and add a > missing test case for QmpOutputVisitor. > > Signed-off-by: Michael Roth > Signed-off-by: Luiz Capitulino Reviewed-by: Anthony Liguori Regards, Anthony Liguori > --- > qapi/qmp-input-visitor.c | 4 +- > qapi/qmp-output-visitor.c | 20 +++++++++++++++--- > scripts/qapi-visit.py | 4 +- > test-visitor.c | 48 +++++++++++++++++++++++++++++++++++++------- > 4 files changed, 60 insertions(+), 16 deletions(-) > > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c > index fcf8bf9..8cbc0ab 100644 > --- a/qapi/qmp-input-visitor.c > +++ b/qapi/qmp-input-visitor.c > @@ -144,8 +144,6 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, > } > (*list)->next = entry; > } > - *list = entry; > - > > return entry; > } > @@ -240,9 +238,11 @@ static void qmp_input_type_enum(Visitor *v, int *obj, const char *strings[], > > if (strings[value] == NULL) { > error_set(errp, QERR_INVALID_PARAMETER, name ? name : "null"); > + g_free(enum_str); > return; > } > > + g_free(enum_str); > *obj = value; > } > > diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c > index 4419a31..d67724e 100644 > --- a/qapi/qmp-output-visitor.c > +++ b/qapi/qmp-output-visitor.c > @@ -20,6 +20,7 @@ > typedef struct QStackEntry > { > QObject *value; > + bool is_list_head; > QTAILQ_ENTRY(QStackEntry) node; > } QStackEntry; > > @@ -45,6 +46,9 @@ static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value) > QStackEntry *e = g_malloc0(sizeof(*e)); > > e->value = value; > + if (qobject_type(e->value) == QTYPE_QLIST) { > + e->is_list_head = true; > + } > QTAILQ_INSERT_HEAD(&qov->stack, e, node); > } > > @@ -122,12 +126,20 @@ static void qmp_output_start_list(Visitor *v, const char *name, Error **errp) > qmp_output_push(qov, list); > } > > -static GenericList *qmp_output_next_list(Visitor *v, GenericList **list, > +static GenericList *qmp_output_next_list(Visitor *v, GenericList **listp, > Error **errp) > { > - GenericList *retval = *list; > - *list = retval->next; > - return retval; > + GenericList *list = *listp; > + QmpOutputVisitor *qov = to_qov(v); > + QStackEntry *e = QTAILQ_FIRST(&qov->stack); > + > + assert(e); > + if (e->is_list_head) { > + e->is_list_head = false; > + return list; > + } > + > + return list ? list->next : NULL; > } > > static void qmp_output_end_list(Visitor *v, Error **errp) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 252230e..62de83d 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -79,11 +79,11 @@ def generate_visit_list(name, members): > > void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp) > { > - GenericList *i; > + GenericList *i, **head = (GenericList **)obj; > > visit_start_list(m, name, errp); > > - for (i = visit_next_list(m, (GenericList **)obj, errp); i; i = visit_next_list(m,&i, errp)) { > + for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m,&i, errp)) { > %(name)sList *native_i = (%(name)sList *)i; > visit_type_%(name)s(m,&native_i->value, NULL, errp); > } > diff --git a/test-visitor.c b/test-visitor.c > index b7717de..847ce14 100644 > --- a/test-visitor.c > +++ b/test-visitor.c > @@ -27,11 +27,11 @@ static void visit_type_TestStruct(Visitor *v, TestStruct **obj, const char *name > > static void visit_type_TestStructList(Visitor *m, TestStructList ** obj, const char *name, Error **errp) > { > - GenericList *i; > + GenericList *i, **head = (GenericList **)obj; > > visit_start_list(m, name, errp); > > - for (i = visit_next_list(m, (GenericList **)obj, errp); i; i = visit_next_list(m,&i, errp)) { > + for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m,&i, errp)) { > TestStructList *native_i = (TestStructList *)i; > visit_type_TestStruct(m,&native_i->value, NULL, errp); > } > @@ -50,6 +50,8 @@ static void test_visitor_core(void) > TestStructList *lts = NULL; > Error *err = NULL; > QObject *obj; > + QList *qlist; > + QDict *qdict; > QString *str; > int64_t value = 0; > > @@ -96,7 +98,9 @@ static void test_visitor_core(void) > g_assert(pts->y == 84); > > qobject_decref(obj); > + g_free(pts); > > + /* test list input visitor */ > obj = qobject_from_json("[{'x': 42, 'y': 84}, {'x': 12, 'y': 24}]"); > mi = qmp_input_visitor_new(obj); > v = qmp_input_get_visitor(mi); > @@ -110,14 +114,41 @@ static void test_visitor_core(void) > g_assert(lts->value->x == 42); > g_assert(lts->value->y == 84); > > - lts = lts->next; > - g_assert(lts != NULL); > - g_assert(lts->value->x == 12); > - g_assert(lts->value->y == 24); > + g_assert(lts->next != NULL); > + g_assert(lts->next->value->x == 12); > + g_assert(lts->next->value->y == 24); > + g_assert(lts->next->next == NULL); > > - g_assert(lts->next == NULL); > + qobject_decref(obj); > > + /* test list output visitor */ > + mo = qmp_output_visitor_new(); > + v = qmp_output_get_visitor(mo); > + visit_type_TestStructList(v,<s, NULL,&err); > + if (err) { > + g_error("%s", error_get_pretty(err)); > + } > + obj = qmp_output_get_qobject(mo); > + g_print("obj: %s\n", qstring_get_str(qobject_to_json(obj))); > + > + qlist = qobject_to_qlist(obj); > + assert(qlist); > + obj = qlist_pop(qlist); > + qdict = qobject_to_qdict(obj); > + assert(qdict); > + assert(qdict_get_int(qdict, "x") == 42); > + assert(qdict_get_int(qdict, "y") == 84); > + qobject_decref(obj); > + > + obj = qlist_pop(qlist); > + qdict = qobject_to_qdict(obj); > + assert(qdict); > + assert(qdict_get_int(qdict, "x") == 12); > + assert(qdict_get_int(qdict, "y") == 24); > qobject_decref(obj); > + > + qmp_output_visitor_cleanup(mo); > + QDECREF(qlist); > } > > /* test deep nesting with refs to other user-defined types */ > @@ -286,7 +317,8 @@ static void test_nested_enums(void) > g_assert(nested_enums_cpy->has_enum2 == false); > g_assert(nested_enums_cpy->has_enum4 == true); > > - qobject_decref(obj); > + qmp_output_visitor_cleanup(mo); > + qmp_input_visitor_cleanup(mi); > qapi_free_NestedEnumsOne(nested_enums); > qapi_free_NestedEnumsOne(nested_enums_cpy); > }