Eric Blake <ebl...@redhat.com> writes:
> Rather than generate inline per-member visits, take advantage
> of the 'visit_type_FOO_members()' function for both event and
> command marshalling. This is possible now that implicit
> structs can be visited like any other.
>
> Generated code shrinks accordingly; events initialize a struct
> based on parameters, through a new gen_param_var() helper, like:
>
> |@@ -338,6 +250,9 @@ void qapi_event_send_block_job_error(con
> | QMPEventFuncEmit emit = qmp_event_get_func_emit();
> | QmpOutputVisitor *qov;
> | Visitor *v;
> |+ q_obj_BLOCK_JOB_ERROR_arg param = {
> |+ (char *)device, operation, action
> |+ };
> |
> | if (!emit) {
> | return;
> @@ -351,19 +266,7 @@ void qapi_event_send_block_job_error(con
> | if (err) {
> | goto out;
> | }
> |- visit_type_str(v, "device", (char **)&device, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |- visit_type_IoOperationType(v, "operation", &operation, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |- visit_type_BlockErrorAction(v, "action", &action, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |-out_obj:
> |+ visit_type_q_obj_BLOCK_JOB_ERROR_arg_members(v, ¶m, &err);
> | visit_end_struct(v, err ? NULL : &err);
>
> Notice that the initialization of 'param' has to cast away const
> (just as the old gen_visit_members() had to do): we can't change
> the signature of the user function (which uses 'const char *'), but
> have to assign it to a non-const QAPI object (which requires
> 'char *').
>
> Likewise, command marshalling generates call arguments from a
> stack-allocated struct, rather than a list of local variables:
>
> |@@ -57,26 +57,15 @@ void qmp_marshal_add_fd(QDict *args, QOb
> | QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
> | QapiDeallocVisitor *qdv;
> | Visitor *v;
> |- bool has_fdset_id = false;
> |- int64_t fdset_id = 0;
> |- bool has_opaque = false;
> |- char *opaque = NULL;
> |-
> |- v = qmp_input_get_visitor(qiv);
> |- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
> |- visit_type_int(v, "fdset-id", &fdset_id, &err);
> |- if (err) {
> |- goto out;
> |- }
> |- }
> |- if (visit_optional(v, "opaque", &has_opaque)) {
> |- visit_type_str(v, "opaque", &opaque, &err);
> |- if (err) {
> |- goto out;
> |- }
> |+ q_obj_add_fd_arg qapi = {0};
Let's calls this arg.
> |+
> |+ v = qmp_input_get_visitor(qiv);
> |+ visit_type_q_obj_add_fd_arg_members(v, &qapi, &err);
> |+ if (err) {
> |+ goto out;
> | }
> |
> |- retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, &err);
> |+ retval = qmp_add_fd(qapi.has_fdset_id, qapi.fdset_id, qapi.has_opaque,
> qapi.opaque, &err);
> | if (err) {
> | goto out;
> | }
> |@@ -88,12 +77,7 @@ out:
> | qmp_input_visitor_cleanup(qiv);
> | qdv = qapi_dealloc_visitor_new();
> | v = qapi_dealloc_get_visitor(qdv);
> |- if (visit_optional(v, "fdset-id", &has_fdset_id)) {
> |- visit_type_int(v, "fdset-id", &fdset_id, NULL);
> |- }
> |- if (visit_optional(v, "opaque", &has_opaque)) {
> |- visit_type_str(v, "opaque", &opaque, NULL);
> |- }
> |+ visit_type_q_obj_add_fd_arg_members(v, &qapi, NULL);
> | qapi_dealloc_visitor_cleanup(qdv);
> | }
>
> For the marshaller, it has the nice side effect of eliminating a
> chance of collision between argument QMP names and local variables.
>
> This patch also paves the way for some followup simplifications
> in the generator, in subsequent patches.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v5: move qapi.py:gen_struct_init() to qapi-event.py:gen_param_var(),
> improve commit message
> v4: new patch
> ---
> scripts/qapi-commands.py | 28 ++++++++++++----------------
> scripts/qapi-event.py | 40 +++++++++++++++++++++++++++++++---------
> 2 files changed, 43 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 3784f33..5ffc381 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -33,8 +33,8 @@ def gen_call(name, arg_type, ret_type):
> assert not arg_type.variants
> for memb in arg_type.members:
> if memb.optional:
> - argstr += 'has_%s, ' % c_name(memb.name)
> - argstr += '%s, ' % c_name(memb.name)
> + argstr += 'qapi.has_%s, ' % c_name(memb.name)
> + argstr += 'qapi.%s, ' % c_name(memb.name)
>
> lhs = ''
> if ret_type:
> @@ -71,21 +71,10 @@ def gen_marshal_vars(arg_type, ret_type):
> QmpInputVisitor *qiv = qmp_input_visitor_new_strict(QOBJECT(args));
> QapiDeallocVisitor *qdv;
> Visitor *v;
> -''')
> + %(c_name)s qapi = {0};
>
> - for memb in arg_type.members:
> - if memb.optional:
> - ret += mcgen('''
> - bool has_%(c_name)s = false;
> ''',
> - c_name=c_name(memb.name))
> - ret += mcgen('''
> - %(c_type)s %(c_name)s = %(c_null)s;
> -''',
> - c_name=c_name(memb.name),
> - c_type=memb.type.c_type(),
> - c_null=memb.type.c_null())
> - ret += '\n'
> + c_name=arg_type.c_name())
> else:
> ret += mcgen('''
>
> @@ -107,17 +96,24 @@ def gen_marshal_input_visit(arg_type, dealloc=False):
> qdv = qapi_dealloc_visitor_new();
> v = qapi_dealloc_get_visitor(qdv);
> ''')
> + errp = 'NULL'
> else:
> ret += mcgen('''
> v = qmp_input_get_visitor(qiv);
> ''')
> + errp = '&err'
>
> - ret += gen_visit_members(arg_type.members, skiperr=dealloc)
> + ret += mcgen('''
> + visit_type_%(c_name)s_members(v, &qapi, %(errp)s);
> +''',
> + c_name=arg_type.c_name(), errp=errp)
>
> if dealloc:
> ret += mcgen('''
> qapi_dealloc_visitor_cleanup(qdv);
> ''')
> + else:
> + ret += gen_err_check()
> return ret
>
>
> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> index 02c9556..c125ecb 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -28,6 +28,30 @@ def gen_event_send_decl(name, arg_type):
> proto=gen_event_send_proto(name, arg_type))
>
>
> +# Declare and initialize an object 'qapi' using parameters from gen_params()
> +def gen_param_var(typ):
> + assert not typ.variants
> + ret = mcgen('''
> + %(c_name)s param = {
> +''',
> + c_name=typ.c_name())
> + sep = ' '
> + for memb in typ.members:
> + ret += sep
> + sep = ', '
> + if memb.optional:
> + ret += 'has_' + c_name(memb.name) + sep
> + if memb.type.name == 'str':
> + # Cast away const added in gen_params()
> + ret += '(char *)'
> + ret += c_name(memb.name)
> + ret += mcgen('''
> +
> + };
> +''')
> + return ret
> +
> +
> def gen_event_send(name, arg_type):
> # FIXME: Our declaration of local variables (and of 'errp' in the
> # parameter list) can collide with exploded members of the event's
> @@ -50,6 +74,7 @@ def gen_event_send(name, arg_type):
> QmpOutputVisitor *qov;
> Visitor *v;
> ''')
> + ret += gen_param_var(arg_type)
>
> ret += mcgen('''
>
> @@ -62,25 +87,22 @@ def gen_event_send(name, arg_type):
> name=name)
>
> if arg_type and arg_type.members:
> - assert not arg_type.variants
Already asserted by new gen_param_var(). Thanks for paying attention to
details.
> ret += mcgen('''
> qov = qmp_output_visitor_new();
> v = qmp_output_get_visitor(qov);
>
> visit_start_struct(v, "%(name)s", NULL, 0, &err);
> -''',
> - name=name)
> - ret += gen_err_check()
> - ret += gen_visit_members(arg_type.members, need_cast=True,
> - label='out_obj')
> - ret += mcgen('''
> -out_obj:
> + if (err) {
> + goto out;
> + }
> + visit_type_%(c_name)s_members(v, ¶m, &err);
> visit_end_struct(v, err ? NULL : &err);
> if (err) {
> goto out;
> }
> qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
> -''')
> +''',
> + name=name, c_name=arg_type.c_name())
>
> ret += mcgen('''
> emit(%(c_enum)s, qmp, &err);
