[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code |
Date: |
Thu, 10 Mar 2016 13:14:19 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 03/10/2016 11:50 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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.
>>
>
> We're not "deferring an assertion to qdict_put_obj()", we're dropping a
> dead one: qmp_output_get_qobject() never returns null.
Oh, good point; I can improve the commit message.
>
> I figure the assertion dates back to the time when it still did. Back
> then, getting null here meant we screwed up.
>
> I just searched the code for similarly dead assertions. Found one in
> qapi_clone_InputEvent(), and serveral more in test-qmp-output-visitor.c.
Speaking of that, I have a patch pending (but not yet posted) that adds
a clone visitor, so that we don't need qapi_clone_InputEvent() (it's
rather wasteful to convert into and back out of QObject when you can
just directly clone).
>
> There's also an error return in qapi_copy_SocketAddress(). Useless?
And that's the other hand-rolled clone that also gets nuked by my patch.
Some obvious copy-and-paste between the two.
> Should check for qnull instead?
Not necessary; we can't return qnull unless we visit nothing (or, when
my visit_type_null() lands, if we explicitly ask for it), but these
callers are visiting something that is not null.
>> %(proto)s
>> {
>> QDict *qmp;
>> Error *err = NULL;
>> - QMPEventFuncEmit emit;
>> + QMPEventFuncEmit emit = qmp_event_get_func_emit();
>> ''',
>> proto=gen_event_send_proto(name, arg_type))
>>
>> @@ -43,16 +49,13 @@ def gen_event_send(name, arg_type):
>> ret += mcgen('''
>> QmpOutputVisitor *qov;
>> Visitor *v;
>> - QObject *obj;
>> -
>
> Please keep the blank line here...
>
>> ''')
>>
>> ret += mcgen('''
>> - emit = qmp_event_get_func_emit();
>> +
>
> ... instead of adding it here.
Except that the next patch added one more declaration after Visitor *v,
but not in direct text, where keeping the blank line unmoved would
require splitting the mcgen() call into two parts. Or I could do ret +=
'\n'.
>
>> if (!emit) {
>> return;
>> }
>> -
>
> Let's keep this one.
Okay.
>
>> qmp = qmp_event_build_dict("%(name)s");
>>
>> ''',
>> @@ -76,11 +79,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));
>> ''')
>>
>> ret += mcgen('''
>
> Small improvements are welcome, too :)
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v5 00/14] easier unboxed visits/qapi implicit types, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 02/14] qapi: Fix command with named empty argument type, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 01/14] qapi: Assert in places where variants are not handled, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 03/14] qapi: Make c_type() more OO-like, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 08/14] qapi-commands: Inline single-use helpers of gen_marshal(), Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 10/14] qapi: Drop unused c_null(), Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 06/14] qapi-event: Slightly shrink generated code, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 07/14] qapi: Utilize implicit struct visits, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 12/14] qapi: Make BlockdevOptions doc example closer to reality, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 09/14] qapi: Inline gen_visit_members() into lone caller, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 05/14] qapi: Emit implicit structs in generated C, Eric Blake, 2016/03/09
- [Qemu-devel] [PATCH v5 04/14] qapi: Adjust names of implicit types, Eric Blake, 2016/03/09