[Qemu-devel] [PATCH v8 11/16] qapi-event: Reduce chance of collision with event data

[Eric Blake]

When an unboxed event has accompanying data, we are exposing all
of its members alongside our local variables in the generated
qapi_event_send_FOO() function.  So far, we haven't hit a
collision, but it may be a matter of time before someone wants
to name a QMP data element 'err' or similar.  We can separate
the names by making the public function (qapi_event_send_FOO())
a shell that boxes things up without having to worry about
collisions, then calls into a new worker function
(do_qapi_event_send_FOO()) that gets generated to look like what
we already output for boxed events.

There is still a chance for collision with 'errp' (if that happens,
tweak c_name() to rename a QMP member 'errp' into the C member
'q_errp'), and with 'param' (if that happens, tweak gen_event_send()
and gen_param_var() to name the temporary variable 'q_param').  But
with the division done here, the real worker function no longer has
to worry about collisions.

Adding the new wrapper now means that we need .c_type() for an
anonymous type given as data to an event, because that type is used
in the signature of the new helper function, so we have to relax
an assertion in QAPISchemaObjectType.

The generated file is unchanged for events without data, and for
boxed events; for unboxed events, the changes are as follows:

|-void qapi_event_send_migration(MigrationStatus status, Error **errp)
|+static void do_qapi_event_send_migration(q_obj_MIGRATION_arg *arg, Error 
**errp)
| {
|     QDict *qmp;
|     Error *err = NULL;
|     QMPEventFuncEmit emit;
|     QObject *obj;
|     Visitor *v;
|-    q_obj_MIGRATION_arg param = {
|-        status
|-    };
|
|     emit = qmp_event_get_func_emit();
|     if (!emit) {
...
|     if (err) {
|         goto out;
|     }
|-    visit_type_q_obj_MIGRATION_arg_members(v, &param, &err);
|+    visit_type_q_obj_MIGRATION_arg_members(v, arg, &err);
|     if (!err) {
|         visit_check_struct(v, &err);
...
|+void qapi_event_send_migration(MigrationStatus status, Error **errp)
|+{
|+    q_obj_MIGRATION_arg param = {
|+        status
|+    };
|+    do_qapi_event_send_migration(&param, errp);
|+}
|+

Signed-off-by: Eric Blake <address@hidden>

---
v8: don't open-code for boxed types, improve commit message,
s/intro/prefix/
v7: new patch
---
 scripts/qapi.py       |  1 -
 scripts/qapi-event.py | 45 ++++++++++++++++++++++++++-------------------
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 48263c4..e051892 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1019,7 +1019,6 @@ class QAPISchemaObjectType(QAPISchemaType):
         return QAPISchemaType.c_name(self)

     def c_type(self):
-        assert not self.is_implicit()
         return c_name(self.name) + pointer_suffix

     def c_unboxed_type(self):
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 2cab588..b2da0a9 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -14,8 +14,13 @@
 from qapi import *


-def gen_event_send_proto(name, arg_type, box):
-    return 'void qapi_event_send_%(c_name)s(%(param)s)' % {
+def gen_event_send_proto(name, arg_type, box, impl=False):
+    prefix='void '
+    if impl and arg_type and not box:
+        box = True
+        prefix='static void do_'
+    return '%(prefix)sqapi_event_send_%(c_name)s(%(param)s)' % {
+        'prefix': prefix,
         'c_name': c_name(name.lower()),
         'param': gen_params(arg_type, box, 'Error **errp')}

@@ -49,21 +54,12 @@ def gen_param_var(typ):

     };
 ''')
-    if not typ.is_implicit():
-        ret += mcgen('''
-    %(c_name)s *arg = &param;
-''',
-                c_name=typ.c_name())
     return ret


 def gen_event_send(name, arg_type, box):
-    # 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.
+    if not arg_type or arg_type.is_empty():
+        assert not box
     ret = mcgen('''

 %(proto)s
@@ -72,17 +68,13 @@ def gen_event_send(name, arg_type, box):
     Error *err = NULL;
     QMPEventFuncEmit emit;
 ''',
-                proto=gen_event_send_proto(name, arg_type, box))
+                proto=gen_event_send_proto(name, arg_type, box, True))

     if arg_type and not arg_type.is_empty():
         ret += mcgen('''
     QObject *obj;
     Visitor *v;
 ''')
-        if not box:
-            ret += gen_param_var(arg_type)
-    else:
-        assert not box

     ret += mcgen('''

@@ -112,7 +104,7 @@ def gen_event_send(name, arg_type, box):
     if (err) {
         goto out;
     }
-    visit_type_%(c_name)s_members(v, &param, &err);
+    visit_type_%(c_name)s_members(v, arg, &err);
     if (!err) {
         visit_check_struct(v, &err);
     }
@@ -144,6 +136,21 @@ out:
     QDECREF(qmp);
 }
 ''')
+
+    if arg_type and not box:
+        ret += mcgen('''
+
+%(proto)s
+{
+''',
+                     proto=gen_event_send_proto(name, arg_type, box))
+        ret += gen_param_var(arg_type)
+        ret += mcgen('''
+    do_qapi_event_send_%(c_name)s(&param, errp);
+}
+''',
+                     c_name=c_name(name.lower()))
+
     return ret


-- 
2.5.5