Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> Slightly rearrange the code in gen_event_send() for less generated
> output, by initializing 'emit' sooner, deferring an assertion
> to qdict_put_obj, and dropping a now-unused 'obj' local variable.
>
> While at it, document a FIXME related to the potential for a
> compiler naming collision - if the user ever creates a QAPI event
> whose name matches 'errp' or one of our local variables (like
> 'emit'), we'll have to revisit how we generate functions to
> avoid the problem.

I think this should go into the next patch, where we actually fix the
same collision in command marshallers.

> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> |     QDict *qmp;
> |     Error *err = NULL;
> |-    QMPEventFuncEmit emit;
> |+    QMPEventFuncEmit emit = qmp_event_get_func_emit();
> |     QmpOutputVisitor *qov;
> |     Visitor *v;
> |-    QObject *obj;
> |
> |-    emit = qmp_event_get_func_emit();
> |     if (!emit) {
> |         return;
> |     }
> |-
> |     qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
> |
> |     qov = qmp_output_visitor_new();
> |@@ -53,11 +50,7 @@ out_obj:
> |     if (err) {
> |         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:

This is actually two separate simplifications.  One is initializing emit
instead of assigning to it:

> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> |     QDict *qmp;
> |     Error *err = NULL;
> |-    QMPEventFuncEmit emit;
> |+    QMPEventFuncEmit emit = qmp_event_get_func_emit();
> |     QmpOutputVisitor *qov;
> |     Visitor *v;
> |     QObject *obj;
> |
> |-    emit = qmp_event_get_func_emit();
> |     if (!emit) {
> |         return;
> |     }
> |-
> |     qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
> |
> |     qov = qmp_output_visitor_new();

I'm afraid I don't like it, because it separates
qmp_event_get_func_emit() from its check.

A more likable simplification would be dropping the indirection
outright: emit is always monitor_qapi_event_queue(), except in
test-qmp-event.  That's a rather weak excuse for complicating things
with an indirection.  Let's not upset this series with it, though.

The other simplification is burying the dead check of
qmp_output_get_qobject()'s value:

> |@@ -25,16 +25,13 @@ void qapi_event_send_acpi_device_ost(ACP
> | {
> |     QDict *qmp;
> |     Error *err = NULL;
> |     QMPEventFuncEmit emit;
> |     QmpOutputVisitor *qov;
> |     Visitor *v;
> |-    QObject *obj;
> |
> |     emit = qmp_event_get_func_emit();
> |@@ -53,11 +50,7 @@ out_obj:
> |     if (err) {
> |         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:

Yes, please.

Patch shrinks to just this:

>From bfaa962b04f5365289c46dadb0c078a13d52eae8 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 9 Mar 2016 17:55:27 -0700
Subject: [PATCH 06/14] qapi-event: Drop qmp_output_get_qobject() null check

qmp_output_get_qobject() was changed never to return null some time
ago, 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 <eblake@redhat.com>
---
 scripts/qapi-event.py | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index c03cb78..27af206 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('''
-- 
2.4.3