Eric Blake <ebl...@redhat.com> writes:
> On 03/03/2017 06:32 AM, Markus Armbruster wrote:
>> The command registry encapsulates a single command list. Give the
>> functions using it a parameter instead. Define suitable command lists
>> in monitor, guest agent and test-qmp-commands.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> include/qapi/qmp/dispatch.h | 22 ++++++++++++++--------
>> monitor.c | 31 +++++++++++++++++--------------
>> qapi/qmp-dispatch.c | 17 +++++++++++++----
>> qapi/qmp-registry.c | 37 ++++++++++++++++++-------------------
>> qga/commands.c | 2 +-
>> qga/guest-agent-core.h | 2 ++
>> qga/main.c | 19 ++++++++++---------
>> scripts/qapi-commands.py | 16 ++++++++++------
>> tests/test-qmp-commands.c | 12 +++++++-----
>> 9 files changed, 92 insertions(+), 66 deletions(-)
>>
>
>> +++ b/qapi/qmp-dispatch.c
>> @@ -67,7 +67,11 @@ static QDict *qmp_dispatch_check_obj(const QObject
>> *request, Error **errp)
>> return dict;
>> }
>>
>> -static QObject *do_qmp_dispatch(QObject *request, Error **errp)
>> +volatile QmpCommand *save_cmd;
>
> A comment why volatile is necessary would be nice...
Uh...
>> +QmpCommand cmd2;
>> +
>> +static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request,
>> + Error **errp)
>> {
>> Error *local_err = NULL;
>> const char *command;
>> @@ -81,7 +85,7 @@ static QObject *do_qmp_dispatch(QObject *request, Error
>> **errp)
>> }
>>
>> command = qdict_get_str(dict, "execute");
>> - cmd = qmp_find_command(command);
>> + cmd = qmp_find_command(cmds, command);
>> if (cmd == NULL) {
>> error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
>> "The command %s has not been found", command);
>> @@ -93,6 +97,9 @@ static QObject *do_qmp_dispatch(QObject *request, Error
>> **errp)
>> return NULL;
>> }
>>
>> + assert(!cmd->options & QCO_NO_SUCCESS_RESP);
>> + save_cmd = cmd;
>> + cmd2 = *cmd;
>> if (!qdict_haskey(dict, "arguments")) {
>> args = qdict_new();
>> } else {
>> @@ -111,6 +118,8 @@ static QObject *do_qmp_dispatch(QObject *request, Error
>> **errp)
>>
>> QDECREF(args);
>>
>> + assert(!cmd->options & QCO_NO_SUCCESS_RESP);
>> + assert(ret || local_err);
>
> ...or is this leftovers from your debugging?
Corret. I'll drop it.
> The rest of the patch looks fine; it is converting a global variable
> into a per-instance variable.
Squashing in the obvious garbage collection. May I add your R-by?
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 95a0f48..72827a3 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -67,9 +67,6 @@ static QDict *qmp_dispatch_check_obj(const QObject *request,
Error **errp)
return dict;
}
-volatile QmpCommand *save_cmd;
-QmpCommand cmd2;
-
static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject *request,
Error **errp)
{
@@ -97,9 +94,6 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds, QObject
*request,
return NULL;
}
- assert(!cmd->options & QCO_NO_SUCCESS_RESP);
- save_cmd = cmd;
- cmd2 = *cmd;
if (!qdict_haskey(dict, "arguments")) {
args = qdict_new();
} else {
@@ -118,8 +112,6 @@ static QObject *do_qmp_dispatch(QmpCommandList *cmds,
QObject *request,
QDECREF(args);
- assert(!cmd->options & QCO_NO_SUCCESS_RESP);
- assert(ret || local_err);
return ret;
}
