Message ID | 1317221085-5825-11-git-send-email-lcapitulino@redhat.com |
---|---|
State | New |
Headers | show |
On 09/28/2011 09:44 AM, Luiz Capitulino wrote: > From: Michael Roth<mdroth@linux.vnet.ibm.com> > > 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<mdroth@linux.vnet.ibm.com> > Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com> 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); > }
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); }