OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

qapi: Add parameter to visit_end_*

Browse Source 
Rather than making the dealloc visitor track of stack of pointers
remembered during visit_start_* in order to free them during
visit_end_*, it's a lot easier to just make all callers pass the
same pointer to visit_end_*.  The generated code has access to the
same pointer, while all other users are doing virtual walks and
can pass NULL.  The dealloc visitor is then greatly simplified.

All three visit_end_*() functions intentionally take a void**,
even though the visit_start_*() functions differ between void**,
GenericList**, and GenericAlternate**.  This is done for several
reasons: when doing a virtual walk, passing NULL doesn't care
what the type is, but when doing a generated walk, we already
have to cast the caller's specific FOO* to call visit_start,
while using void** lets us use visit_end without a cast. Also,
an upcoming patch will add a clone visitor that wants to use
the same implementation for all three visit_end callbacks,
which is made easier if all three share the same signature.

For visitors with already track per-object state (the QMP visitors
via a stack, and the string visitors which do not allow nesting),
add an assertion that the caller is indeed passing the same
pointer to paired calls.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-4-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This commit is contained in:
Eric Blake 2016-06-09 10:48:34 -06:00 committed by Markus Armbruster
parent 911ee36d41
commit 1158bb2a05
20 changed files with 86 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
block/crypto.c
View File

@ -222,7 +222,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
visit_check_struct(opts_get_visitor(ov), &local_err);
}
visit_end_struct(opts_get_visitor(ov));
visit_end_struct(opts_get_visitor(ov), NULL);
out:
if (local_err) {
@ -269,7 +269,7 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
visit_check_struct(opts_get_visitor(ov), &local_err);
}
visit_end_struct(opts_get_visitor(ov));
visit_end_struct(opts_get_visitor(ov), NULL);
out:
if (local_err) {

8
docs/qapi-code-gen.txt
View File

@ -904,7 +904,7 @@ Example:
}
visit_check_struct(v, &err);
out_obj:
visit_end_struct(v);
visit_end_struct(v, (void **)obj);
if (err && visit_is_input(v)) {
qapi_free_UserDefOne(*obj);
*obj = NULL;
@ -932,7 +932,7 @@ Example:
}
}
visit_end_list(v);
visit_end_list(v, (void **)obj);
if (err && visit_is_input(v)) {
qapi_free_UserDefOneList(*obj);
*obj = NULL;
@ -1022,7 +1022,7 @@ Example:
if (!err) {
visit_check_struct(v, &err);
}
visit_end_struct(v);
visit_end_struct(v, NULL);
if (err) {
goto out;
}
@ -1041,7 +1041,7 @@ Example:
v = qapi_dealloc_get_visitor(qdv);
visit_start_struct(v, NULL, NULL, 0, NULL);
visit_type_UserDefOneList(v, "arg1", &arg1, NULL);
visit_end_struct(v);
visit_end_struct(v, NULL);
qapi_dealloc_visitor_cleanup(qdv);
}

4
hw/ppc/spapr_drc.c
View File

@ -300,7 +300,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
/* shouldn't ever see an FDT_END_NODE before FDT_BEGIN_NODE */
g_assert(fdt_depth > 0);
visit_check_struct(v, &err);
visit_end_struct(v);
visit_end_struct(v, NULL);
if (err) {
error_propagate(errp, err);
return;
@ -323,7 +323,7 @@ static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
return;
}
}
visit_end_list(v);
visit_end_list(v, NULL);
break;
}
default:

4
hw/virtio/virtio-balloon.c
View File

@ -139,13 +139,13 @@ static void balloon_stats_get_all(Object *obj, Visitor *v, const char *name,
}
visit_check_struct(v, &err);
out_nested:
visit_end_struct(v);
visit_end_struct(v, NULL);
if (!err) {
visit_check_struct(v, &err);
}
out_end:
visit_end_struct(v);
visit_end_struct(v, NULL);
out:
error_propagate(errp, err);
}

6
include/qapi/visitor-impl.h
View File

@ -47,7 +47,7 @@ struct Visitor
void (*check_struct)(Visitor *v, Error **errp);
/* Must be set to visit structs */
void (*end_struct)(Visitor *v);
void (*end_struct)(Visitor *v, void **obj);
/* Must be set; implementations may require @list to be non-null,
* but must document it. */
@ -58,7 +58,7 @@ struct Visitor
GenericList *(*next_list)(Visitor *v, GenericList *tail, size_t size);
/* Must be set */
void (*end_list)(Visitor *v);
void (*end_list)(Visitor *v, void **list);
/* Must be set by input and dealloc visitors to visit alternates;
* optional for output visitors. */
@ -67,7 +67,7 @@ struct Visitor
bool promote_int, Error **errp);
/* Optional, needed for dealloc visitor */
void (*end_alternate)(Visitor *v);
void (*end_alternate)(Visitor *v, void **obj);
/* Must be set */
void (*type_int64)(Visitor *v, const char *name, int64_t *obj,

32
include/qapi/visitor.h
View File

@ -193,12 +193,12 @@
* goto outlist;
* }
* outlist:
* visit_end_list(v);
* visit_end_list(v, NULL);
* if (!err) {
* visit_check_struct(v, &err);
* }
* outobj:
* visit_end_struct(v);
* visit_end_struct(v, NULL);
* out:
* error_propagate(errp, err);
* ...clean up v...
@ -242,8 +242,8 @@ typedef struct GenericAlternate {
* After visit_start_struct() succeeds, the caller may visit its
* members one after the other, passing the member's name and address
* within the struct. Finally, visit_end_struct() needs to be called
* to clean up, even if intermediate visits fail. See the examples
* above.
* with the same @obj to clean up, even if intermediate visits fail.
* See the examples above.
*
* FIXME Should this be named visit_start_object, since it is also
* used for QAPI unions, and maps to JSON objects?
@ -267,12 +267,14 @@ void visit_check_struct(Visitor *v, Error **errp);
/*
* Complete an object visit started earlier.
*
* @obj must match what was passed to the paired visit_start_struct().
*
* Must be called after any successful use of visit_start_struct(),
* even if intermediate processing was skipped due to errors, to allow
* the backend to release any resources. Destroying the visitor early
* behaves as if this was implicitly called.
*/
void visit_end_struct(Visitor *v);
void visit_end_struct(Visitor *v, void **obj);
/*** Visiting lists ***/
@ -299,8 +301,9 @@ void visit_end_struct(Visitor *v);
* visit (where @obj is NULL) uses other means. For each list
* element, call the appropriate visit_type_FOO() with name set to
* NULL and obj set to the address of the value member of the list
* element. Finally, visit_end_list() needs to be called to clean up,
* even if intermediate visits fail. See the examples above.
* element. Finally, visit_end_list() needs to be called with the
* same @list to clean up, even if intermediate visits fail. See the
* examples above.
*/
void visit_start_list(Visitor *v, const char *name, GenericList **list,
size_t size, Error **errp);
@ -324,12 +327,14 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size);
/*
* Complete a list visit started earlier.
*
* @list must match what was passed to the paired visit_start_list().
*
* Must be called after any successful use of visit_start_list(), even
* if intermediate processing was skipped due to errors, to allow the
* backend to release any resources. Destroying the visitor early
* behaves as if this was implicitly called.
*/
void visit_end_list(Visitor *v);
void visit_end_list(Visitor *v, void **list);
/*** Visiting alternates ***/
@ -347,8 +352,9 @@ void visit_end_list(Visitor *v);
*
* If @promote_int, treat integers as QTYPE_FLOAT.
*
* If successful, this must be paired with visit_end_alternate() to
* clean up, even if visiting the contents of the alternate fails.
* If successful, this must be paired with visit_end_alternate() with
* the same @obj to clean up, even if visiting the contents of the
* alternate fails.
*/
void visit_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
@ -357,15 +363,15 @@ void visit_start_alternate(Visitor *v, const char *name,
/*
* Finish visiting an alternate type.
*
* @obj must match what was passed to the paired visit_start_alternate().
*
* Must be called after any successful use of visit_start_alternate(),
* even if intermediate processing was skipped due to errors, to allow
* the backend to release any resources. Destroying the visitor early
* behaves as if this was implicitly called.
*
* TODO: Should all the visit_end_* interfaces take obj parameter, so
* that dealloc visitor need not track what was passed in visit_start?
*/
void visit_end_alternate(Visitor *v);
void visit_end_alternate(Visitor *v, void **obj);
/*** Other helpers ***/

4
qapi/opts-visitor.c
View File

@ -180,7 +180,7 @@ opts_check_struct(Visitor *v, Error **errp)
static void
opts_end_struct(Visitor *v)
opts_end_struct(Visitor *v, void **obj)
{
OptsVisitor *ov = to_ov(v);
@ -273,7 +273,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
static void
opts_end_list(Visitor *v)
opts_end_list(Visitor *v, void **obj)
{
OptsVisitor *ov = to_ov(v);

47
qapi/qapi-dealloc-visitor.c
View File

@ -19,53 +19,18 @@
#include "qapi/qmp/types.h"
#include "qapi/visitor-impl.h"
typedef struct StackEntry
{
void *value;
QTAILQ_ENTRY(StackEntry) node;
} StackEntry;
struct QapiDeallocVisitor
{
Visitor visitor;
QTAILQ_HEAD(, StackEntry) stack;
};
static QapiDeallocVisitor *to_qov(Visitor *v)
{
return container_of(v, QapiDeallocVisitor, visitor);
}
static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value)
{
StackEntry *e = g_malloc0(sizeof(*e));
e->value = value;
QTAILQ_INSERT_HEAD(&qov->stack, e, node);
}
static void *qapi_dealloc_pop(QapiDeallocVisitor *qov)
{
StackEntry *e = QTAILQ_FIRST(&qov->stack);
QObject *value;
QTAILQ_REMOVE(&qov->stack, e, node);
value = e->value;
g_free(e);
return value;
}
static void qapi_dealloc_start_struct(Visitor *v, const char *name, void **obj,
size_t unused, Error **errp)
{
QapiDeallocVisitor *qov = to_qov(v);
qapi_dealloc_push(qov, obj);
}
static void qapi_dealloc_end_struct(Visitor *v)
static void qapi_dealloc_end_struct(Visitor *v, void **obj)
{
QapiDeallocVisitor *qov = to_qov(v);
void **obj = qapi_dealloc_pop(qov);
if (obj) {
g_free(*obj);
}
@ -75,14 +40,10 @@ static void qapi_dealloc_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
bool promote_int, Error **errp)
{
QapiDeallocVisitor *qov = to_qov(v);
qapi_dealloc_push(qov, obj);
}
static void qapi_dealloc_end_alternate(Visitor *v)
static void qapi_dealloc_end_alternate(Visitor *v, void **obj)
{
QapiDeallocVisitor *qov = to_qov(v);
void **obj = qapi_dealloc_pop(qov);
if (obj) {
g_free(*obj);
}
@ -102,7 +63,7 @@ static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList *tail,
return next;
}
static void qapi_dealloc_end_list(Visitor *v)
static void qapi_dealloc_end_list(Visitor *v, void **obj)
{
}
@ -178,7 +139,5 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
v->visitor.type_any = qapi_dealloc_type_anything;
v->visitor.type_null = qapi_dealloc_type_null;
QTAILQ_INIT(&v->stack);
return v;
}

12
qapi/qapi-visit-core.c
View File

@ -43,9 +43,9 @@ void visit_check_struct(Visitor *v, Error **errp)
}
}
void visit_end_struct(Visitor *v)
void visit_end_struct(Visitor *v, void **obj)
{
v->end_struct(v);
v->end_struct(v, obj);
}
void visit_start_list(Visitor *v, const char *name, GenericList **list,
@ -67,9 +67,9 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size)
return v->next_list(v, tail, size);
}
void visit_end_list(Visitor *v)
void visit_end_list(Visitor *v, void **obj)
{
v->end_list(v);
v->end_list(v, obj);
}
void visit_start_alternate(Visitor *v, const char *name,
@ -89,10 +89,10 @@ void visit_start_alternate(Visitor *v, const char *name,
error_propagate(errp, err);
}
void visit_end_alternate(Visitor *v)
void visit_end_alternate(Visitor *v, void **obj)
{
if (v->end_alternate) {
v->end_alternate(v);
v->end_alternate(v, obj);
}
}

11
qapi/qmp-input-visitor.c
View File

@ -26,6 +26,7 @@
typedef struct StackObject
{
QObject *obj; /* Object being visited */
void *qapi; /* sanity check that caller uses same pointer */
GHashTable *h; /* If obj is dict: unvisited keys */
const QListEntry *entry; /* If obj is list: unvisited tail */
@ -96,7 +97,7 @@ static void qdict_add_key(const char *key, QObject *obj, void *opaque)
}
static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
Error **errp)
void *qapi, Error **errp)
{
GHashTable *h;
StackObject *tos = &qiv->stack[qiv->nb_stack];
@ -108,6 +109,7 @@ static const QListEntry *qmp_input_push(QmpInputVisitor *qiv, QObject *obj,
}
tos->obj = obj;
tos->qapi = qapi;
assert(!tos->h);
assert(!tos->entry);
@ -145,12 +147,13 @@ static void qmp_input_check_struct(Visitor *v, Error **errp)
}
}
static void qmp_input_pop(Visitor *v)
static void qmp_input_pop(Visitor *v, void **obj)
{
QmpInputVisitor *qiv = to_qiv(v);
StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
assert(qiv->nb_stack > 0);
assert(tos->qapi == obj);
if (qiv->strict) {
GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
@ -179,7 +182,7 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj,
return;
}
qmp_input_push(qiv, qobj, &err);
qmp_input_push(qiv, qobj, obj, &err);
if (err) {
error_propagate(errp, err);
return;
@ -207,7 +210,7 @@ static void qmp_input_start_list(Visitor *v, const char *name,
return;
}
entry = qmp_input_push(qiv, qobj, errp);
entry = qmp_input_push(qiv, qobj, list, errp);
if (list) {
if (entry) {
*list = g_malloc0(size);

23
qapi/qmp-output-visitor.c
View File

@ -22,6 +22,7 @@
typedef struct QStackEntry
{
QObject *value;
void *qapi; /* sanity check that caller uses same pointer */
QTAILQ_ENTRY(QStackEntry) node;
} QStackEntry;
@ -36,7 +37,8 @@ struct QmpOutputVisitor
#define qmp_output_add(qov, name, value) \
qmp_output_add_obj(qov, name, QOBJECT(value))
#define qmp_output_push(qov, value) qmp_output_push_obj(qov, QOBJECT(value))
#define qmp_output_push(qov, value, qapi) \
qmp_output_push_obj(qov, QOBJECT(value), qapi)
static QmpOutputVisitor *to_qov(Visitor *v)
{
@ -44,23 +46,26 @@ static QmpOutputVisitor *to_qov(Visitor *v)
}
/* Push @value onto the stack of current QObjects being built */
static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value)
static void qmp_output_push_obj(QmpOutputVisitor *qov, QObject *value,
void *qapi)
{
QStackEntry *e = g_malloc0(sizeof(*e));
assert(qov->root);
assert(value);
e->value = value;
e->qapi = qapi;
QTAILQ_INSERT_HEAD(&qov->stack, e, node);
}
/* Pop a value off the stack of QObjects being built, and return it. */
static QObject *qmp_output_pop(QmpOutputVisitor *qov)
static QObject *qmp_output_pop(QmpOutputVisitor *qov, void *qapi)
{
QStackEntry *e = QTAILQ_FIRST(&qov->stack);
QObject *value;
assert(e);
assert(e->qapi == qapi);
QTAILQ_REMOVE(&qov->stack, e, node);
value = e->value;
assert(value);
@ -104,13 +109,13 @@ static void qmp_output_start_struct(Visitor *v, const char *name, void **obj,
QDict *dict = qdict_new();
qmp_output_add(qov, name, dict);
qmp_output_push(qov, dict);
qmp_output_push(qov, dict, obj);
}
static void qmp_output_end_struct(Visitor *v)
static void qmp_output_end_struct(Visitor *v, void **obj)
{
QmpOutputVisitor *qov = to_qov(v);
QObject *value = qmp_output_pop(qov);
QObject *value = qmp_output_pop(qov, obj);
assert(qobject_type(value) == QTYPE_QDICT);
}
@ -122,7 +127,7 @@ static void qmp_output_start_list(Visitor *v, const char *name,
QList *list = qlist_new();
qmp_output_add(qov, name, list);
qmp_output_push(qov, list);
qmp_output_push(qov, list, listp);
}
static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
@ -131,10 +136,10 @@ static GenericList *qmp_output_next_list(Visitor *v, GenericList *tail,
return tail->next;
}
static void qmp_output_end_list(Visitor *v)
static void qmp_output_end_list(Visitor *v, void **obj)
{
QmpOutputVisitor *qov = to_qov(v);
QObject *value = qmp_output_pop(qov);
QObject *value = qmp_output_pop(qov, obj);
assert(qobject_type(value) == QTYPE_QLIST);
}

7
qapi/string-input-visitor.c
View File

@ -30,6 +30,7 @@ struct StringInputVisitor
int64_t cur;
const char *string;
void *list; /* Only needed for sanity checking the caller */
};
static StringInputVisitor *to_siv(Visitor *v)
@ -120,6 +121,7 @@ start_list(Visitor *v, const char *name, GenericList **list, size_t size,
/* We don't support visits without a list */
assert(list);
siv->list = list;
if (parse_str(siv, name, errp) < 0) {
*list = NULL;
@ -168,8 +170,11 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
return tail->next;
}
static void end_list(Visitor *v)
static void end_list(Visitor *v, void **obj)
{
StringInputVisitor *siv = to_siv(v);
assert(siv->list == obj);
}
static void parse_type_int64(Visitor *v, const char *name, int64_t *obj,

5
qapi/string-output-visitor.c
View File

@ -64,6 +64,7 @@ struct StringOutputVisitor
uint64_t u;
} range_start, range_end;
GList *ranges;
void *list; /* Only needed for sanity checking the caller */
};
static StringOutputVisitor *to_sov(Visitor *v)
@ -274,6 +275,7 @@ start_list(Visitor *v, const char *name, GenericList **list, size_t size,
assert(sov->list_mode == LM_NONE);
/* We don't support visits without a list */
assert(list);
sov->list = list;
/* List handling is only needed if there are at least two elements */
if (*list && (*list)->next) {
sov->list_mode = LM_STARTED;
@ -291,10 +293,11 @@ static GenericList *next_list(Visitor *v, GenericList *tail, size_t size)
return ret;
}
static void end_list(Visitor *v)
static void end_list(Visitor *v, void **obj)
{
StringOutputVisitor *sov = to_sov(v);
assert(sov->list == obj);
assert(sov->list_mode == LM_STARTED ||
sov->list_mode == LM_END ||
sov->list_mode == LM_NONE ||

2
qom/object.c
View File

@ -2044,7 +2044,7 @@ static void property_get_tm(Object *obj, Visitor *v, const char *name,
}
visit_check_struct(v, &err);
out_end:
visit_end_struct(v);
visit_end_struct(v, NULL);
out:
error_propagate(errp, err);

4
qom/object_interfaces.c
View File

@ -71,7 +71,7 @@ Object *user_creatable_add(const QDict *qdict,
obj = user_creatable_add_type(type, id, pdict, v, &local_err);
out_visit:
visit_end_struct(v);
visit_end_struct(v, NULL);
out:
QDECREF(pdict);
@ -127,7 +127,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
if (!local_err) {
visit_check_struct(v, &local_err);
}
visit_end_struct(v);
visit_end_struct(v, NULL);
if (local_err) {
goto out;
}

4
scripts/qapi-commands.py
View File

@ -129,7 +129,7 @@ def gen_marshal(name, arg_type, ret_type):
if (!err) {
visit_check_struct(v, &err);
}
visit_end_struct(v);
visit_end_struct(v, NULL);
if (err) {
goto out;
}
@ -160,7 +160,7 @@ out:
v = qapi_dealloc_get_visitor(qdv);
visit_start_struct(v, NULL, NULL, 0, NULL);
visit_type_%(c_name)s_members(v, &arg, NULL);
visit_end_struct(v);
visit_end_struct(v, NULL);
qapi_dealloc_visitor_cleanup(qdv);
''',
c_name=arg_type.c_name())

2
scripts/qapi-event.py
View File

@ -101,7 +101,7 @@ def gen_event_send(name, arg_type):
if (!err) {
visit_check_struct(v, &err);
}
visit_end_struct(v);
visit_end_struct(v, NULL);
if (err) {
goto out;
}

8
scripts/qapi-visit.py
View File

@ -129,7 +129,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
}
}
visit_end_list(v);
visit_end_list(v, (void **)obj);
if (err && visit_is_input(v)) {
qapi_free_%(c_name)s(*obj);
*obj = NULL;
@ -194,7 +194,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
if (!err) {
visit_check_struct(v, &err);
}
visit_end_struct(v);
visit_end_struct(v, NULL);
''',
c_type=var.type.c_name(),
c_name=c_name(var.name))
@ -216,7 +216,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
"%(name)s");
}
out_obj:
visit_end_alternate(v);
visit_end_alternate(v, (void **)obj);
if (err && visit_is_input(v)) {
qapi_free_%(c_name)s(*obj);
*obj = NULL;
@ -250,7 +250,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error
}
visit_check_struct(v, &err);
out_obj:
visit_end_struct(v);
visit_end_struct(v, (void **)obj);
if (err && visit_is_input(v)) {
qapi_free_%(c_name)s(*obj);
*obj = NULL;

2
tests/test-qmp-input-visitor.c
View File

@ -304,7 +304,7 @@ static void test_visitor_in_null(TestInputVisitorData *data,
visit_type_null(v, "b", &err);
error_free_or_abort(&err);
visit_check_struct(v, &error_abort);
visit_end_struct(v);
visit_end_struct(v, NULL);
}
static void test_visitor_in_union_flat(TestInputVisitorData *data,

2
tests/test-qmp-output-visitor.c
View File

@ -499,7 +499,7 @@ static void test_visitor_out_null(TestOutputVisitorData *data,
visit_start_struct(data->ov, NULL, NULL, 0, &error_abort);
visit_type_null(data->ov, "a", &error_abort);
visit_check_struct(data->ov, &error_abort);
visit_end_struct(data->ov);
visit_end_struct(data->ov, NULL);
arg = qmp_output_get_qobject(data->qov);
g_assert(qobject_type(arg) == QTYPE_QDICT);
qdict = qobject_to_qdict(arg);