Kevin Wolf <[email protected]> writes:
> Am 14.09.2020 um 17:10 hat Markus Armbruster geschrieben:
>> Kevin Wolf <[email protected]> writes:
>>
>> > The correct way to set the current monitor for a coroutine handler will
>> > be different than for a blocking handler, so monitor_set_cur() needs to
>> > be called in qmp_dispatch().
>> >
>> > Signed-off-by: Kevin Wolf <[email protected]>
>> > ---
>> > include/qapi/qmp/dispatch.h | 3 ++-
>> > monitor/qmp.c | 8 +-------
>> > qapi/qmp-dispatch.c | 8 +++++++-
>> > qga/main.c | 2 +-
>> > stubs/monitor-core.c | 5 +++++
>> > tests/test-qmp-cmds.c | 6 +++---
>> > 6 files changed, 19 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
>> > index 5a9cf82472..0c2f467028 100644
>> > --- a/include/qapi/qmp/dispatch.h
>> > +++ b/include/qapi/qmp/dispatch.h
>> > @@ -14,6 +14,7 @@
>> > #ifndef QAPI_QMP_DISPATCH_H
>> > #define QAPI_QMP_DISPATCH_H
>> >
>> > +#include "monitor/monitor.h"
>> > #include "qemu/queue.h"
>> >
>> > typedef void (QmpCommandFunc)(QDict *, QObject **, Error **);
>> > @@ -49,7 +50,7 @@ const char *qmp_command_name(const QmpCommand *cmd);
>> > bool qmp_has_success_response(const QmpCommand *cmd);
>> > QDict *qmp_error_response(Error *err);
>> > QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request,
>> > - bool allow_oob);
>> > + bool allow_oob, Monitor *cur_mon);
>> > bool qmp_is_oob(const QDict *dict);
>> >
>> > typedef void (*qmp_cmd_callback_fn)(const QmpCommand *cmd, void *opaque);
>> > diff --git a/monitor/qmp.c b/monitor/qmp.c
>> > index 8469970c69..922fdb5541 100644
>> > --- a/monitor/qmp.c
>> > +++ b/monitor/qmp.c
>> > @@ -135,16 +135,10 @@ static void monitor_qmp_respond(MonitorQMP *mon,
>> > QDict *rsp)
>> >
>> > static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
>> > {
>> > - Monitor *old_mon;
>> > QDict *rsp;
>> > QDict *error;
>> >
>> > - old_mon = monitor_set_cur(&mon->common);
>> > - assert(old_mon == NULL);
>> > -
>> > - rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon));
>> > -
>> > - monitor_set_cur(NULL);
>> > + rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon),
>> > &mon->common);
>>
>> Long line. Happy to wrap it in my tree. A few more in PATCH 08-11.
>
> It's 79 characters. Should be fine even with your local deviation from
> the coding style to require less than that for comments?
Let me rephrase my remark.
For me,
rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon),
&mon->common);
is significantly easier to read than
rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon), &mon->common);
Would you mind me wrapping this line in my tree?
A few more in PATCH 08-11.