Eric Blake <ebl...@redhat.com> 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.
