From 29f6bd15eb8a55ed37b2a443f7275b3d134eb2b2 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Mar 2016 16:48:26 -0600 Subject: qapi: Assert in places where variants are not handled We are getting closer to the point where we could use one union as the base or variant type within another union type (as long as there are no collisions between any possible combination of member names allowed across all discriminator choices). But until we get to that point, it is worth asserting that variants are not present in places where we are not prepared to handle them: when exploding a type into a parameter list, we do not expect variants. The qapi.py code is already checking this, via the older check_type() method; but someday we hope to get rid of that and move checking into QAPISchema*.check(). The two asserts added here make sure any refactoring still catches problems, and makes it locally obvious why we can iterate over only type.members without worrying about type.variants. Signed-off-by: Eric Blake Message-Id: <1458254921-17042-2-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-event.py | 1 + 1 file changed, 1 insertion(+) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index fb579dd098..c03cb78a8e 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -59,6 +59,7 @@ def gen_event_send(name, arg_type): name=name) if arg_type and arg_type.members: + assert not arg_type.variants ret += mcgen(''' qov = qmp_output_visitor_new(); v = qmp_output_get_visitor(qov); -- cgit 1.4.1 From 8df59565d2c27dec8c96a2090f0eb73303efce14 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Mar 2016 16:48:31 -0600 Subject: qapi-event: Drop qmp_output_get_qobject() null check qmp_output_get_qobject() was changed never to return null some time ago (in commit 6c2f9a15), but the qapi_event_send_FOO() functions still check. Clean that up: |@@ -28,7 +28,6 @@ void qapi_event_send_acpi_device_ost(ACP | QMPEventFuncEmit emit; | QmpOutputVisitor *qov; | Visitor *v; |- QObject *obj; | | emit = qmp_event_get_func_emit(); | if (!emit) { |@@ -54,10 +53,7 @@ out_obj: | goto out; | } | |- obj = qmp_output_get_qobject(qov); |- g_assert(obj); |- |- qdict_put_obj(qmp, "data", obj); |+ qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov)); | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err); | | out: Signed-off-by: Eric Blake Message-Id: <1458254921-17042-7-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-event.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index c03cb78a8e..27af206a9d 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -43,7 +43,6 @@ def gen_event_send(name, arg_type): ret += mcgen(''' QmpOutputVisitor *qov; Visitor *v; - QObject *obj; ''') @@ -77,10 +76,7 @@ out_obj: goto out; } - obj = qmp_output_get_qobject(qov); - g_assert(obj); - - qdict_put_obj(qmp, "data", obj); + qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov)); ''') ret += mcgen(''' -- cgit 1.4.1 From 0949e95b48e30715e157cabbc59dcb0ed912d3ff Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Mar 2016 16:48:32 -0600 Subject: qapi-event: Utilize implicit struct visits Rather than generate inline per-member visits, take advantage of the 'visit_type_FOO_members()' function for emitting events. This is possible now that implicit structs can be visited like any other. Generated code shrinks accordingly; by initializing 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 *'). While touching this, document with a FIXME comment that there is still a potential collision between QMP members and our choice of local variable names within qapi_event_send_FOO(). This patch also paves the way for some followup simplifications in the generator, in subsequent patches. Signed-off-by: Eric Blake Message-Id: <1458254921-17042-8-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-event.py | 48 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 10 deletions(-) (limited to 'scripts/qapi-event.py') diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py index 27af206a9d..9b5c5b535d 100644 --- a/scripts/qapi-event.py +++ b/scripts/qapi-event.py @@ -28,7 +28,37 @@ 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 + # data type passed in as parameters. If this collision ever hits in + # practice, we can rename our local variables with a leading _ prefix, + # or split the code into a wrapper function that creates a boxed + # 'param' object then calls another to do the real work. ret = mcgen(''' %(proto)s @@ -43,10 +73,11 @@ def gen_event_send(name, arg_type): ret += mcgen(''' QmpOutputVisitor *qov; Visitor *v; - ''') + ret += gen_param_var(arg_type) ret += mcgen(''' + emit = qmp_event_get_func_emit(); if (!emit) { return; @@ -58,26 +89,23 @@ def gen_event_send(name, arg_type): name=name) if arg_type and arg_type.members: - assert not arg_type.variants 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); -- cgit 1.4.1