qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v4 05/10] qapi: Utilize implicit struct visits
Date: Tue, 08 Mar 2016 20:21:52 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 03/08/2016 11:09 AM, Markus Armbruster wrote:
>
>> This patch actually fixes a similar issue in the qmp_marshal_FOO()
>> functions.
>
> Indeed, and I didn't even realize it. I'll add that to the commit message :)
>
>> 
>> To keep ignoring it in the qapi_event_send_BAR() functions is okay.
>> It's fairly easy to fix now, though: split them into two, so that the
>> outer half does nothing but parameter wrapping.  For instance,
>> 
>>     void qapi_event_send_block_io_error(const char *device, IoOperationType 
>> operation, BlockErrorAction action, bool has_nospace, bool nospace, const 
>> char *reason, Error **errp)
>>     {
>>         QDict *qmp;
>>         Error *err = NULL;
>>         QMPEventFuncEmit emit;
>>         QmpOutputVisitor *qov;
>>         Visitor *v;
>>         QObject *obj;
>>         _obj_BLOCK_IO_ERROR_arg param = {
>>             (char *)device, operation, action, has_nospace, nospace, (char 
>> *)reason
>>         };
>> 
>>         [do stuff...]
>>     }
>> 
>> becomes
>> 
>>     static inline void do_event_send_block_io_error(_obj_BLOCK_IO_ERROR_arg 
>> param, Error **errp)
>>     {
>>         QDict *qmp;
>>         Error *err = NULL;
>>         QMPEventFuncEmit emit;
>>         QmpOutputVisitor *qov;
>>         Visitor *v;
>>         QObject *obj;
>> 
>>         [do stuff...]
>>     }
>> 
>>     void qapi_event_send_block_io_error(const char *device, IoOperationType 
>> operation, BlockErrorAction action, bool has_nospace, bool nospace, const 
>> char *reason, Error **errp)
>
> Still means we can't have 'errp' as a QMP member of the error, without
> some sort of renaming.  Again, not worth worrying about until we
> actually want to avoid the collision.

Rename it to q_errp.

>>     {
>>         do_event_send_block_io_error((_obj_BLOCK_IO_ERROR_arg){
>>                 (char *)device, operation, action, has_nospace, nospace, 
>> (char *)reason
>>             }, errp);
>>         };
>>     }
>> 
>> Feel free not to do that now, but mark the spot with a comment then.
>> Since it's technically wrong, we could even mark it FIXME.
>
> In fact, I have a patch in a later series [1] that WANTS to let the user
> supply a boxed parameter - at which point, the difference between two
> vs. one function would be whether the user requested boxing.  Sounds
> like I add the FIXME here, and then that series can take care of the
> possible split.
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04394.html

Works for me.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]