Eric Blake <ebl...@redhat.com> writes: > Commit e8316d7 mistakenly passed consume=true
in qmp_input_optional(), right? > when checking if > an optional member was present, but the mistake was silently > ignored since the code happily let us extract a member more than > once. Tighten up the input visitor to ensure that a member is > consumed exactly once. To keep the testsuite happy in the case > of incomplete input, we have to check whether a member exists > in the dictionary before trying to remove it. Sure this is only for the testsuite's benefit? You fix commit e8316d7's incorrect consume=true, don't you? Recommend to mention that explicitly. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v14: no change > v13: no change > v12: new patch > --- > qapi/qmp-input-visitor.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c > index 7ba6d3d..cfaf378 100644 > --- a/qapi/qmp-input-visitor.c > +++ b/qapi/qmp-input-visitor.c > @@ -65,10 +65,12 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv, > /* If we have a name, and we're in a dictionary, then return that > * value. */ > if (name && qobject_type(qobj) == QTYPE_QDICT) { > - if (tos->h && consume) { > - g_hash_table_remove(tos->h, name); > + qobj = qdict_get(qobject_to_qdict(qobj), name); > + if (tos->h && consume && qobj) { > + bool removed = g_hash_table_remove(tos->h, name); > + assert(removed); > } > - return qdict_get(qobject_to_qdict(qobj), name); > + return qobj; > } > > /* If we are in the middle of a list, then return the next element > @@ -338,7 +340,7 @@ static void qmp_input_type_any(Visitor *v, const char > *name, QObject **obj, > static void qmp_input_optional(Visitor *v, const char *name, bool *present) > { > QmpInputVisitor *qiv = to_qiv(v); > - QObject *qobj = qmp_input_get_object(qiv, name, true); > + QObject *qobj = qmp_input_get_object(qiv, name, false); > > if (!qobj) { > *present = false;