[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is adva
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is advanced |
Date: |
Thu, 28 Apr 2016 17:19:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> In the QMP input visitor, visiting a list traverses two objects:
> the QAPI GenericList of the caller (which gets advanced in
> visit_next_list() regardless of this patch), and the QList input
> that we are converting to QAPI. For consistency with QDict
> visits, we want to consume elements from the input QList during
> the visit_type_FOO() for the list element; that is, we want ALL
> the code for consuming an input to live in qmp_input_get_object(),
> rather than having it split according to whether we are visiting
> a dict or a list. Making qmp_input_get_object() the common point
> of consumption will make it easier for a later patch to refactor
> visit_start_list() to cover the GenericList * head of a QAPI list,
> and in turn will get rid of the 'first' flag (which lived in
> qmp_input_next_list() pre-patch, and is hoisted to StackObject
> by this patch).
>
> This patch is therefore shifting the post-condition use of
> 'entry' from:
>
> start_list next_list type_ELT ... next_list type_ELT end_list
> entry NULL 1st elt 1st elt last elt last elt gone
>
> where type_ELT() returns (entry ? entry : 1st elt) and next_list() steps
> entry
>
> to this usage:
>
> start_list next_list type_ELT ... next_list type_ELT end_list
> entry 1st elt 1nd elt 2nd elt last elt NULL gone
>
> where type_ELT() steps entry and returns the old entry, and next_list()
> leaves entry alone.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v15: rebase, improve commit message, hoist root handling earlier
> so that this patch is more pinpointed
> v14: no change
> v13: no change
> v12: new patch
> ---
> qapi/qmp-input-visitor.c | 30 ++++++++++++++++--------------
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 6409a83..eb77adc 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -29,6 +29,7 @@ typedef struct StackObject
>
> GHashTable *h; /* If obj is dict: unvisited keys */
> const QListEntry *entry; /* If obj is list: unvisited tail */
> + bool first; /* If obj is list: will next_list() visit head
> */
I like documenting bools with the yes/no question they answer.
Questions end with a question mark, though :)
Perhaps: /* If obj is list: next_list() not yet called? */
Since this will go away soon, feel free not to change anything here.
> } StackObject;
>
> struct QmpInputVisitor
> @@ -80,7 +81,11 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
> } else {
> assert(qobject_type(qobj) == QTYPE_QLIST);
> assert(!name);
> + assert(!tos->first);
> ret = qlist_entry_obj(tos->entry);
> + if (consume) {
> + tos->entry = qlist_next(tos->entry);
> + }
> }
>
> return ret;
> @@ -104,13 +109,16 @@ static void qmp_input_push(QmpInputVisitor *qiv,
> QObject *obj, Error **errp)
> }
>
> tos->obj = obj;
> - tos->entry = NULL;
> - tos->h = NULL;
> + assert(!tos->h);
> + assert(!tos->entry);
>
> if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
> h = g_hash_table_new(g_str_hash, g_str_equal);
> qdict_iter(qobject_to_qdict(obj), qdict_add_key, h);
> tos->h = h;
> + } else if (qobject_type(obj) == QTYPE_QLIST) {
> + tos->entry = qlist_first(qobject_to_qlist(obj));
> + tos->first = true;
> }
>
> qiv->nb_stack++;
> @@ -119,10 +127,11 @@ static void qmp_input_push(QmpInputVisitor *qiv,
> QObject *obj, Error **errp)
>
> static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
> {
> + StackObject *tos = &qiv->stack[qiv->nb_stack - 1];
> assert(qiv->nb_stack > 0);
>
> if (qiv->strict) {
> - GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
> + GHashTable * const top_ht = tos->h;
Let's clean this up to *const while we're touching it. Can do on
commit.
> if (top_ht) {
> GHashTableIter iter;
> const char *key;
> @@ -133,6 +142,7 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error
> **errp)
> }
> g_hash_table_unref(top_ht);
> }
> + tos->h = NULL;
> }
>
> qiv->nb_stack--;
> @@ -192,23 +202,15 @@ static GenericList *qmp_input_next_list(Visitor *v,
> GenericList **list,
> QmpInputVisitor *qiv = to_qiv(v);
> GenericList *entry;
> StackObject *so = &qiv->stack[qiv->nb_stack - 1];
> - bool first;
>
> - if (so->entry == NULL) {
> - so->entry = qlist_first(qobject_to_qlist(so->obj));
> - first = true;
> - } else {
> - so->entry = qlist_next(so->entry);
> - first = false;
> - }
> -
> - if (so->entry == NULL) {
> + if (!so->entry) {
> return NULL;
> }
>
> entry = g_malloc0(size);
> - if (first) {
> + if (so->first) {
> *list = entry;
> + so->first = false;
> } else {
> (*list)->next = entry;
> }
This stuff never fails to confuse me. But now there's hope it's mostly
because the *old* code is confusing.
- [Qemu-devel] [PATCH v15 00/23] qapi visitor cleanups (post-introspection cleanups subset E), Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 07/23] qapi-commands: Wrap argument visit in visit_start_struct, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 03/23] qmp: Drop dead command->type, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 06/23] qmp-input: Don't consume input when checking has_member, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 09/23] qom: Wrap prop visit in visit_start_struct, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 04/23] qmp-input: Clean up stack handling, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is advanced, Eric Blake, 2016/04/27
- Re: [Qemu-devel] [PATCH v15 11/23] qmp-input: Refactor when list is advanced,
Markus Armbruster <=
- [Qemu-devel] [PATCH v15 13/23] tests: Add check-qnull, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 02/23] qapi: Guarantee NULL obj on input visitor callback error, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 02A/23] fixup! qapi: Guarantee NULL obj on input visitor callback error, Eric Blake, 2016/04/28
- [Qemu-devel] [PATCH v15 14/23] qapi: Add visit_type_null() visitor, Eric Blake, 2016/04/27
- [Qemu-devel] [PATCH v15 05/23] qapi: Use strict QMP input visitor in more places, Eric Blake, 2016/04/27