[Qemu-devel] [PATCH v7 10/15] qapi-event: Reduce chance of collision with event data

[Eric Blake]

When an event has data that is not boxed, we are exposing all of
its members alongside our local variables.  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 a shell that creates a
simple wrapper, then calls a worker that operates on only the
boxed version and thus has no user-supplied names to worry about
in naming its local variables.  For boxed events, we don't need
the wrapper.

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.

The generated file changes 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 <ebl...@redhat.com>

---
v7: new patch
---
 scripts/qapi.py       |  1 -
 scripts/qapi-event.py | 47 ++++++++++++++++++++++++++---------------------
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6742e7a..7d568d9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1016,7 +1016,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 b8ca8c8..fe4e50d 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):
+    intro='void '
+    if impl and arg_type and not box:
+        box = True
+        intro='static void do_'
+    return '%(intro)sqapi_event_send_%(c_name)s(%(param)s)' % {
+        'intro': intro,
         'c_name': c_name(name.lower()),
         'param': gen_params(arg_type, box, 'Error **errp')}

@@ -53,12 +58,8 @@ def gen_param_var(typ):


 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():
+        box = False
     ret = mcgen('''

 %(proto)s
@@ -67,15 +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)

     ret += mcgen('''

@@ -93,20 +92,11 @@ def gen_event_send(name, arg_type, box):
         ret += mcgen('''
     v = qmp_output_visitor_new(&obj);

-''')
-
-        if box:
-            ret += mcgen('''
-    visit_type_%(c_name)s(v, NULL, &arg, &err);
-''',
-                         c_name=arg_type.c_name(), name=arg_type.name)
-        else:
-            ret += mcgen('''
     visit_start_struct(v, "%(name)s", NULL, 0, &err);
     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);
     }
@@ -136,6 +126,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