Hi
On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake <ebl...@redhat.com> wrote:
> All other successful clients of visit_start_struct() were paired
> with an unconditional visit_end_struct(); but the generated
> code for events was relying on qmp_output_visitor_cleanup() to
> work on an incomplete visit. Alter the code to guarantee that
> the struct is completed, which will make a future patch to
> split visit_end_struct() easier to reason about. While at it,
> drop some assertions and comments that are not present in other
> uses of the qmp output visitor, rearrange the declaration to
> make it easier for a future patch to introduce the notion of
> a boxed event visit, and pass NULL rather than "" as the 'kind'
> parameter (matching most other uses where obj is NULL).
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v8: no change
> v7: place earlier in series, adjust handling of 'kind'
> v6: new patch
>
> If desired, I can defer the hunk re-ordering the declaration of
> obj to later in the series where it actually comes in handy.
> ---
> scripts/qapi-event.py | 14 ++++++--------
> scripts/qapi.py | 5 +++--
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 720486f..e37c07a 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -41,9 +41,9 @@ def gen_event_send(name, arg_type):
>
> if arg_type and arg_type.members:
> ret += mcgen('''
> + QObject *obj;
> QmpOutputVisitor *qov;
> Visitor *v;
> - QObject *obj;
This looks like churning code.
>
> ''')
>
> @@ -61,25 +61,23 @@ def gen_event_send(name, arg_type):
> if arg_type and arg_type.members:
> ret += mcgen('''
> qov = qmp_output_visitor_new();
> - g_assert(qov);
> -
> v = qmp_output_get_visitor(qov);
> - g_assert(v);
>
> - /* Fake visit, as if all members are under a structure */
> - visit_start_struct(v, NULL, "", "%(name)s", 0, &err);
> + visit_start_struct(v, NULL, NULL, "%(name)s", 0, &err);
The comment seemed somewhat useful to me.
> ''',
> name=name)
> ret += gen_err_check()
> - ret += gen_visit_fields(arg_type.members, need_cast=True)
> + ret += gen_visit_fields(arg_type.members, need_cast=True,
> + label='out_obj')
> ret += mcgen('''
> +out_obj:
> visit_end_struct(v, &err);
> if (err) {
> goto out;
> }
>
> obj = qmp_output_get_qobject(qov);
> - g_assert(obj != NULL);
> + g_assert(obj);
>
> qdict_put_obj(qmp, "data", obj);
> ''')
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 7dec611..497eaba 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1636,7 +1636,8 @@ def gen_err_check(label='out', skiperr=False):
> label=label)
>
>
> -def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
> +def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False,
> + label='out'):
> ret = ''
> if skiperr:
> errparg = 'NULL'
> @@ -1664,7 +1665,7 @@ def gen_visit_fields(members, prefix='',
> need_cast=False, skiperr=False):
> c_type=memb.type.c_name(), prefix=prefix, cast=cast,
> c_name=c_name(memb.name), name=memb.name,
> errp=errparg)
> - ret += gen_err_check(skiperr=skiperr)
> + ret += gen_err_check(skiperr=skiperr, label=label)
>
> if memb.optional:
> pop_indent()
> --
> 2.4.3
>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
--
Marc-André Lureau
